At the end of the day, what I want is a consistent API that I can code against
in the head of master that's treated like a contract. As an API user outside
of the ATC repo it's incredibly frustrating to have my stuff break all the
time. It basically encourages never developing using the latest API versions
(regardless of how they're defined and even then things still break
retroactively) or a non-official OSS release alltogether. It's a catch22 to be
forced to either not vendor the go/python/bash libraries which leads to
constant develop/recompile/deploys in lockstep with ATC or vendor and still
have to do these things when stuff breaks anyway in the API. Really debating
the native client libraries at all is just a red herring because the root issue
is the HTTP API itself which is the real thing to care about since not all
integrations use one of the client libraries, nor can be forced to do so, and
may require a rigid API definition.
Jonathan G
On 4/18/19, 10:12 AM, "Rawlin Peters" <rawlin.pet...@gmail.com> wrote:
> The UPDATE statements need modified to fix #3497 even if we get rid of
> versioning. Unless we decide to permanently break all clients older than
> the newest server field, with every new server upgrade. The only other
> option is to fix the updates. Unless you know of a way to fix missing
> fields without changing the update statements, that I'm not seeing?
By removing minor versioning, only certain clients that don't handle
new unknown fields would potentially be broken, and I believe only the
TO Go client has that problem in our repo. However, the TO Go client
happens to use the same Go structs as traffic_ops_golang, so whenever
new fields are added to the API, all the client has to do is recompile
with the up-to-date structs. Unless we made breaking changes to the
client, in most cases all that would be needed for those clients is a
recompile. Traffic Portal, the Python TO client, and I'm pretty sure
the Java TO client all handle unknown fields properly.
Without minor versions, #3497 would not even an issue. It's only an
issue because of the attempt to support minor versioning. If we just
support the major version, all client requests would be treated as v1,
and there would only ever be one SQL UPDATE statement per major
version. We wouldn't need to "upgrade" 1.2 requests into a 1.4 struct
(thus preventing the bug in #3497) by selecting and inserting all 1.4
values from the DB into the struct before handling the request or
dynamically generating the SQL UPDATE statement to use based on the
requested minor version.
> So, this solution actually gives us
> this bug fix almost for free. All that's required is another small
function
> to iterate over the object fields to create the update query. It's by far
> the easiest and simplest fix for #3497; unless we also permanently break
> all older clients on every server upgrade along with the minor version
> removal.
Switching all the endpoints over to your "apiver" library would not be
as trivial to implement or remove as you make it sound. It would
require lots of added API test coverage and a non-trivial amount of
code modifications to all API endpoints. Certain UPDATE queries might
be easy to generate from a given struct if the struct only uses a
single table, but I don't think something like that would work for a
field like `cachegroup.LocalizationMethods` which doesn't come from
the cachegroups table and is updated separately from the rest of the
cachegroup fields.
- Rawlin
