> 2c. Reflection is a tradeoff between debuggability and copypasta bugs
> 3. As I said in the issue, it's an unfortunate mistake we weren't specific 
> enough but it was permissible and therefore falls under 1a.  I generally 
> agree with strong typing, and even going forward we should increment our API 
> major rev and just document that as a new requirement going forward in API 
> changes.
> 4. There's some benefit to versioning your objects, but not all languages are 
> smart enough trivially to be able to support more than 1 option and it 
> ignores the routes problem.  I think the better option is the adoption of 
> something like json-schema/swagger that's implementation neutral, but still 
> machine readable. Alternatively, I'm still a fan of GraphQL which has a 
> strong and flexible schema built in as a first class concept and is 
> impossible to have 2 and 3 while not requiring 3rd party validation tools.  
> This results frequently in versionless APIs entirely because you're always 
> required to ask for specifically what you want, are prepared to handle, and 
> should you ask for something that no longer exists the server API is allowed 
> to return an error (in this case you had a bigger problem anyway than API 
> issues).
> 5. I include it only because it highlights issues we have with deployments of 
> ATC components when choosing to ignore 5d and have 3rd-party software 
> dependent on our API that moves faster than our major release cycle.  There 
> are several pro/con to the debate of one-version-to-rule-them-all and source 
> control monorepo, for today this is just what we've chosen to do as a project.

A part of why I split 2 and 3 apart was because in cases of 2, as an API 
consumer I'm choosing when to pay the technical debts that come with newer 
releases.  In the case of 3, that debt is being forced to be paid with each 
endpoint conversion not on a planned timetable.  I'm not arguing it's positive 
change, just unplanned change.

1a is very different than 1b and very important because it provides the path 
for both 3rd parties and ATC components themselves to upgrade smoothly and on 
separate timetabels.  You can't just turn off all of ATC for an upgrade, so 
there's always a period of time where components have to interoperate.  Due to 
1a, at least one of the ATC API versions from a prior ATC release is required 
for 1b.  

I'm not a strong proponent of adding version data into the structs themselves 
because it ignores the other half of the problem being the routes.  
Additionally, if one of the existing major objections is maintaining more code 
and complexity imagine having to support every variant of the struct and 
knowing how as a server to translate between them.  PATCH instead of POST would 
probably be cheaper and safer in the long-run I suspect.  As a client if I 
patch with something the server don't know about, it's allowed to either drop 
or reject.  If a client don't supply all the fields in a patch, that's ok 
(provided it's not a primary key).  That leaves GET, which is where the burden 
would fall to the client implementation to not explode on new or missing fields 
which is why that's in the route.  A lot of the expense in maintaining API 
versions comes with client-side GET and POST matching today.  If they don't 
have to, I could GET with a minor rev but feel confident PATCHing to a major or 
the same major.minor from the GET.

Jonathan G


On 4/19/19, 5:32 PM, "Rawlin Peters" <rawlin.pet...@gmail.com> wrote:

    1. Agree
    2. Removal hasn't really been a problem recently, the major concern is 
addition
      2b. Agree, but I could potentially see a lot of the same issues as
    minor versioning w.r.t. increased complexity in the codebase to
    support PATCH operations
      2c. Rob's apiver library seems like a double-edged sword. It might
    make it easier to support minor versioning, but truly supporting minor
    versioning is a brand new feature that would require specialized
    implementation across the entire API (in addition to the maintenance
    cost of being heavily reflection-based)
    3. I've stated this before, but I think we just have to move forward
    with strong types in the API and communicate that it was really a
    breaking change that API clients need to be aware of. Stringly-typed
    values are bad.
    4. I think this could largely be addressed by including a "Version"
    field in all struct types (what I proposed earlier today on this
    thread). If you get a Version back that's less than what you expected,
    you might be trying to set fields that the API doesn't support yet. If
    Version is greater than what you expected, you might be wiping out
    values in new, unknown fields. If the Version is exactly what you
    expect, you are safe to update at will.
    5. Agree, but this seems mostly unrelated to the topic at hand.
    
    My thoughts on Rob's proposal for only rolling majors for the TC version:
    - seems to go against semantic versioning at the TC-level (i.e. not
    clear that certain releases are just hotfix releases,
    backwards-compatible releases, or releases with major new
    functionality that might be disruptive)
    - seems like a workaround to the issue of minor versioning (i.e. the
    major version is now the minor version)
    - I don't think our TO API version should be independent of the TC
    release version
    - seems like it would unnecessarily break a lot of clients due to
    always incrementing the API major version even when no breaking
    changes have been made to an API.
    
    What do you think about just supporting the "major version promise"
    but providing a Version number in all the TC structs? The Version
    number would essentially be a true, minor version of the struct (even
    incremented every time a new field is added). For clients that might
    cause data loss if not working off the exact same struct versions,
    they would have enough information from the API to decide whether or
    not they should proceed with an update operation. Then, for clients
    that are "round-trip safe" (i.e. don't drop unknown JSON fields on a
    GET), they can totally ignore the Version info and continue to operate
    as they do today.
    
    - Rawlin
    
    On Fri, Apr 19, 2019 at 3:39 PM Gray, Jonathan
    <jonathan_g...@comcast.com> wrote:
    >
    > Also to be sure I'm tracking properly the complex subject here:
    >
    > 1. Today there are two promises:
    >   1a. One major rev back at the project level for all code/api/etc
    >   1b. API versions independently (but probably not stable enough on 
whatever the latest is due to self-discipline/definition)
    >
    > 2.  Primary issues around API stability are the addition and removal of 
routes/fields
    >   2a.  This relates to the discussion around 1b
    >   2b.  A significant portion of this could be mitigated through the 
support of a PATCH operation as opposed to passing full objects around which 
already has inherent race conditions in a multi-user system.
    >   2c.  This is where maintenance costs are potentially very high, but 
might be mitigated somewhat through rob's apiver library.
    >
    > 3.  Secondary issues are around strong versus weak typing
    >   3a. This is the primary source or breakage lately and is the result of 
conversions from perl to go
    >   3b.  This breaks not only promise 1b, but also promise 1a.
    >   3c.  The solution to this is simply to be more diligent in the use of 
existing or new datatypes until such time that a new major revision of either 
1a or 1b is made.  Example use case issue #3304.
    >
    > 4.  Better support for discoverability and compatibility by and between 
API components/consumers
    >   4a.  At the moment there is no way as a client to know which API 
versions are supported by an arbitrary TO instance.  Documented in issue #2872
    >   4b. This leads to failure scenarios wherein TO isn't upgraded first or 
newer clients exist.  I stopped counting when 5 different people internally ran 
into this with the addition of the 1.4 API for us.
    >
    > 5.  There are additional concerns with how we handle 1b with regard to 
master as opposed to OSS releases and promise 1a
    >   5a. There is a lack of formality when it comes to does component X on 
master changeset A work with component Y on master changeset B before it lands 
in OSS Release Q.
    >   5b.  Presently this entails SME reviewing changelogs of component X & Y.
    >   5c.  This is the question SemVer helps with by better defining that 
formality at the API layer.
    >     5c.i.  It's not a total solution because unversioned 
payloads/workflows such as the CRConfig can still cause additional issues.
    >   5d.  This is where today we're relying on our existing monolithic 
repository and one version to rule them all stance so that in theory we never 
ask 5a
    >     5d.i.  This is backed by our current testing procedures.  When 
running TO API tests today it's presumed to use one version inside the CIAB 
environment.
    >
    > I think if we can clarify and agree on this, the questsions around how to 
version technically and ATC-based clients versus 3rd-party clients is mostly 
mitigated.
    >
    > Jonathan G
    >
    >
    > On 4/19/19, 1:26 PM, "Rawlin Peters" <rawlin.pet...@gmail.com> wrote:
    >
    >     How about this:
    >     1. At the Go struct level, every struct for an API endpoint gets a
    >     "Version" field.
    >     2. Every time a backwards-compatible field is added to an API
    >     endpoint, the Version is incremented for that struct (even if a TC
    >     release hasn't been made between increments).
    >     3. This Version field is read-only and included in all GET/PUT/POST
    >     JSON responses.
    >
    >     For the Go TO client, this would allow the client to GET a resource
    >     and check if the resource's version matches the version that the
    >     client is currently using. If the client is just reading certain
    >     fields and only cares to read the fields it already knows about, it
    >     wouldn't have to check the version field at all. If the client wants
    >     to update a resource, it could GET the resource, compare the resource
    >     versions, and make a determination about whether or not the update
    >     would be safe. If the versions match, it can safely update the
    >     resource without risk of data loss. If the versions do NOT match, the
    >     client might choose to error out instead and/or send an email to the
    >     maintainer that it should be recompiled for safety.
    >
    >     This would allow classes of clients that handle unknown fields
    >     properly (e.g. Traffic Portal, Python+Java TO clients) to continue
    >     working as they do today, only having to worry about the API major
    >     version.
    >
    >     For clients that might be more susceptible to data loss due to
    >     addition of unknown fields (e.g. Go TO client), they would have enough
    >     information returned to them by the API in order to know if they can
    >     safely make updates to resources. If a Go TO client is only reading
    >     certain fields and not making updates, it would probably never need to
    >     be recompiled for the entire life of the API major version. If a Go TO
    >     client is updating resources, it only needs recompiled as often as the
    >     APIs its actually using have been updated.
    >
    >     This would allow random, non-standard TO clients to be written and
    >     used for as long as possible before recompilation is required, and
    >     allow the client to build in proper safeguards only where necessary.
    >
    >     - Rawlin
    >
    >     On Fri, Apr 19, 2019 at 12:01 PM Robert Butts <r...@apache.org> wrote:
    >     >
    >     > I'm trying really hard to come up with a solution that addresses 
everyone's
    >     > major concerns. I think we'll have a better product, that everyone 
can live
    >     > with, if we all try to think of solutions and are willing to 
compromise,
    >     > rather than take hard-line approach and refuse to compromise, and 
argue
    >     > until we're all just unhappy, and whatever gets pushed through 
meets a few
    >     > people's concerns and nobody else's. I'd definitely appreciate any 
help in
    >     > that regard.
    >     >
    >     > Sometimes there really are only two options, A or B. But this 
particular
    >     > issue has countless possibilities. We're all smart people, we can 
figure
    >     > something out that addresses everyone's needs and concerns.
    >     >
    >     > What about this idea:
    >     >
    >     > Along the lines of @hbeatty 's suggestion, what if we:
    >     >
    >     > 1. Make the API version match the TC version.
    >     > 2. Always release new TC major versions, never do TC minor version 
releases.
    >     > 3. Support one major version back, in the API and clients.
    >     > 4. New backward-compatible changes require a TC=API major version 
increase.
    >     > 5. OPTIONAL: There should be an API route to get the exact TC 
version (e.g.
    >     > https://.../api/v3/version). (This isn't strictly necessary, but 
it's
    >     > on @hbeatty
    >     > 's list, and I know it's on @jonathan_gray 's, and it's super-easy 
and
    >     > there's no reason not to.)
    >     >
    >     > This:
    >     > 1. Addresses the client version bugs concern: older clients simply 
don't
    >     > work because we don't support them, and newer clients will get the 
"please
    >     > downgrade" response.
    >     > 2. Addresses the code ease-of-writing concern: We only ever have to
    >     > maintain 1 older version in the API, which will typically only be a 
few
    >     > fields on a few endpoints.
    >     > 3. Partially addresses the ease-of-use concern 
(Ops/@jonathan_gray). It
    >     > addresses the scripts-breaking-things problem, but it does make user
    >     > scripts have a hard upgrade deadline. I see this as the biggest 
weakness of
    >     > this idea, and unfortunately I don't see a remedy; if the user-side 
people
    >     > are willing to live with that?
    >     > 4. Patch versions are still ok. This doesn't prevent e.g. 4.0.1 
when we
    >     > find a major bug in a release; just adding new things that would be 
a
    >     > SemVer Minor Version.
    >     >
    >     > Some points:
    >     > 1. Only doing major versions, we'll obviously quickly reach Traffic 
Control
    >     > Version 47. I think that's ok. There's precedent for this, Chrome 
and
    >     > Firefox both do this, Chrome's latest version is 68 and Firefox is 
66. It
    >     > might seem odd, but I don't think there are any big downsides.
    >     > 2. This will make @jonathan_gray 's (/ Ops) life slightly harder, 
having to
    >     > upgrade script clients more frequently. But it prevents the data 
loss risks
    >     > (which I know everyone here doesn't agree with, but some of us do, 
so bear
    >     > with me), and upgrading our maintained clients should be relatively 
simple.
    >     > 2.1 As @hbeatty points out, if we release at a 6-month interval, 
this would
    >     > mean scripts using old clients would be supported for 1 year. We 
could
    >     > optionally support 2 major versions back, if we were willing to 
live with a
    >     > little more server work, to support 2-year-old clients.
    >     >
    >     > Just to be clear, I personally don't like making the API version 
match the
    >     > TC version, for reasons I won't get into here. I also loathe 
Reflection.
    >     > But I can live with those things, if it addresses everyone else's 
concerns.
    >     > This proposal isn't perfect; there is no perfect solution that will 
fulfill
    >     > everyone's ideal. But, is this something we could live with? If 
not, is
    >     > there a way to modify it to address whatever is unacceptable, while 
still
    >     > addressing the major concerns others have? Or is this just right 
out?
    >     >
    >     >
    >     > On Fri, Apr 19, 2019 at 10:43 AM Jeff Elsloo <els...@apache.org> 
wrote:
    >     >
    >     > > Without actually seeing how that would look across the code base, 
the
    >     > > best I can say is maybe. On the surface your proposal seems to 
improve
    >     > > the areas I'm concerned about, but we still have this implicit 
model
    >     > > where the server is responsible for dealing with older clients 
that
    >     > > might not submit all data as expected. This implicitly requires 
us to
    >     > > handle the absence of that data in future APIs and think about 
how any
    >     > > change might impact all client versions across versions of ATC.
    >     > >
    >     > > My concern really amounts to the investment of time required to 
think
    >     > > through and implement changes that may affect the myriad of 
different
    >     > > client/server version combinations. If we remove that from the
    >     > > equation entirely, we have a much simpler API that has a 1:1
    >     > > correspondence with the route and function, and only one way to
    >     > > create/update a $thing (i.e.: a delivery service). I think having 
only
    >     > > one way to create/update a $thing is a much safer way of doing
    >     > > business than continuing to support multiple versions of clients,
    >     > > regardless of how easy that might be with this proposed approach.
    >     > > Unless I'm missing something, the implementation might be 
simplified
    >     > > using this approach but the complexity of solving for the 
combination
    >     > > of client versions still exists which makes it harder to do 
anything
    >     > > when writing API code.
    >     > >
    >     > > So, it isn't a matter of whether this approach is simple enough 
for us
    >     > > to continue with semantic versioning. It's a matter of whether we 
want
    >     > > to have to continue to deal with older clients that prevent us 
from
    >     > > making certain changes in the API because we are afraid of 
breaking
    >     > > that client. I think that's a lot of burden for our small 
development
    >     > > team to shoulder for questionable utility. Viewed from another 
lens,
    >     > > with the semantic versioning approach we are enabling clients to 
be
    >     > > lazy about updating their _unknown and custom_ client code at the 
cost
    >     > > of developer productivity and progress on our project.
    >     > >
    >     > > I'm not saying that semantic versioning is solely to blame for our
    >     > > lack of progress on our migration to Golang, but it's one more 
thing
    >     > > that is slowing us down and definitely hasn't helped improve 
progress.
    >     > > --
    >     > > Thanks,
    >     > > Jeff
    >     > >
    >     > > On Thu, Apr 18, 2019 at 3:23 PM Robert Butts <r...@apache.org> 
wrote:
    >     > > >
    >     > > > >This is about simplifying our code in the API
    >     > > >
    >     > > > @jeff.elsloo That's what the tag solution I proposed does. The 
only
    >     > > > difference from not versioning, is that fields will have a new 
tag,
    >     > > > "NewField *int `json:"newField, db:"new_field", api:"1.5"`, and 
endpoints
    >     > > > will have an extra line, "json := api.NewJSON("1.4")". That's 
it. That
    >     > > > would be the entirety of the API code (or very nearly, Rawlin 
is right, I
    >     > > > haven't implemented it to be 100% sure). The library itself is 
also tiny,
    >     > > > it's ~250 lines of logic in a single file. 500 lines including 
comments
    >     > > and
    >     > > > boilerplate.
    >     > > >
    >     > > > How do you feel about that? Would that be simple enough?
    >     > > >
    >     > > >
    >     > > > On Thu, Apr 18, 2019 at 3:15 PM Fieck, Brennan <
    >     > > brennan_fi...@comcast.com>
    >     > > > wrote:
    >     > > >
    >     > > > > >If you're deploying the head of master, API minor versioning 
doesn't
    >     > > > > really solve that consistent API problem unless we start 
saying that
    >     > > > > every single new field added to an API endpoint is a new 
minor version
    >     > > > > instead of just incrementing an API's version once per TC 
release.
    >     > > > >
    >     > > > > Yeah, you shouldn't expect an active development branch to be 
stable -
    >     > > > > it's the whole reason we have releases at all. We can't 
support
    >     > > something
    >     > > > > that changes potentially a dozen times in a day.
    >     > > > >
    >     > > > > >If someone goes
    >     > > > > to the trouble to understand how our APIs work and develops 
their own
    >     > > > > client code, why is it so unreasonable to expect them to also
    >     > > > > understand how an update of Traffic Ops could impact their 
_custom_
    >     > > > > tooling?
    >     > > > >
    >     > > > > I agree with this so hard. I'd love to just say "TO vX uses 
the vX API,
    >     > > > > major changes to the biggest TC component are a major change 
to TC",
    >     > > but at
    >     > > > > any given time we support and provide bug/security fixes for 
versions
    >     > > X and
    >     > > > > X-1. I'll settle for eliminating minor API versions, though.
    >     > > Developers can
    >     > > > > be expected to understand that changing versions of a thing 
can change
    >     > > > > aspects of the ways in which you can interact with said 
thing. A major
    >     > > > > version change means major changes and a minor version change 
means
    >     > > minor
    >     > > > > changes.
    >     > > > > ________________________________________
    >     > > > > From: Jeff Elsloo <jeff.els...@gmail.com>
    >     > > > > Sent: Thursday, April 18, 2019 2:59 PM
    >     > > > > To: dev@trafficcontrol.apache.org
    >     > > > > Subject: Re: [EXTERNAL] Re: Traffic Ops API versioning issues
    >     > > > >
    >     > > > > > Maybe I'm the only one, and everyone else can vote me out, 
but I
    >     > > don't
    >     > > > > see
    >     > > > > that as acceptable. It's our responsibility as developers to 
create a
    >     > > safe
    >     > > > > user experience, and unacceptable to declare real bugs to be 
the user's
    >     > > > > fault for not using it right. When our Production CDN goes 
down
    >     > > because an
    >     > > > > Ops person used an old client and didn't "just recompile," 
it's not
    >     > > that
    >     > > > > Ops person's fault, it's our fault as Developers, for 
designing a
    >     > > dangerous
    >     > > > > system. Our job is to prevent the CDN from going down, not to 
shift the
    >     > > > > blame when it does.
    >     > > > >
    >     > > > > I don't think this is about shifting blame, safety, or the 
potential
    >     > > > > to crash a CDN. This is about simplifying our code in the API 
and
    >     > > > > making it more maintainable. If we simplify the API, we can 
accelerate
    >     > > > > development and get more things done, and maybe even complete 
this
    >     > > > > Golang migration. Another plus is simplification of routes by
    >     > > > > eliminating versioning means less code and likely more 
stability and
    >     > > > > safety, easier testing, and less developer confusion, in the 
long run.
    >     > > > >
    >     > > > > I think it's unreasonable for us to shoulder the burden and 
cost to
    >     > > > > maintain various API versions because we're afraid we might 
break some
    >     > > > > client out in the wild that might or might not exist. If 
someone goes
    >     > > > > to the trouble to understand how our APIs work and develops 
their own
    >     > > > > client code, why is it so unreasonable to expect them to also
    >     > > > > understand how an update of Traffic Ops could impact their 
_custom_
    >     > > > > tooling?
    >     > > > >
    >     > > > > Obviously we have to hold up our end of the deal and have 
good API
    >     > > > > documentation and change logs. I think the cost of 
maintaining that is
    >     > > > > much less than API versioning given our experience, 
especially after
    >     > > > > we simplify the APIs. We're already doing much of that today.
    >     > > > > --
    >     > > > > Thanks,
    >     > > > > Jeff
    >     > > > >
    >     > > > > On Thu, Apr 18, 2019 at 11:37 AM Robert Butts 
<r...@apache.org> wrote:
    >     > > > > >
    >     > > > > > >Without minor versions, #3497 would not even an issue. 
It's only an
    >     > > > > issue
    >     > > > > > because of the attempt to support minor versioning.
    >     > > > > >
    >     > > > > > That's simply not true. It's exactly the same issue. 
Removing minor
    >     > > > > > versioning just hides the issue. You have declared:
    >     > > > > >
    >     > > > > > >only certain clients that don't handle new unknown fields 
would
    >     > > > > > potentially be broken
    >     > > > > >
    >     > > > > > >all the client has to do is recompile
    >     > > > > >
    >     > > > > > Something doesn't cease to be an issue, because you 
redefine it to
    >     > > be the
    >     > > > > > user's fault. It's exactly the same issue, removing minor 
versions
    >     > > just
    >     > > > > > makes it much more difficult to debug.
    >     > > > > >
    >     > > > > > You're proposing not only removing minor versions, but 
creating data
    >     > > loss
    >     > > > > > and version mismatch bugs, and declaring them to be the 
user's fault.
    >     > > > > >
    >     > > > > > Maybe I'm the only one, and everyone else can vote me out, 
but I
    >     > > don't
    >     > > > > see
    >     > > > > > that as acceptable. It's our responsibility as developers 
to create a
    >     > > > > safe
    >     > > > > > user experience, and unacceptable to declare real bugs to 
be the
    >     > > user's
    >     > > > > > fault for not using it right. When our Production CDN goes 
down
    >     > > because
    >     > > > > an
    >     > > > > > Ops person used an old client and didn't "just recompile," 
it's not
    >     > > that
    >     > > > > > Ops person's fault, it's our fault as Developers, for 
designing a
    >     > > > > dangerous
    >     > > > > > system. Our job is to prevent the CDN from going down, not 
to shift
    >     > > the
    >     > > > > > blame when it does.
    >     > > > > >
    >     > > > > > >Switching all the endpoints over to your "apiver" library 
would not
    >     > > be
    >     > > > > as
    >     > > > > > trivial to implement or remove as you make it sound.
    >     > > > > >
    >     > > > > > Maybe. I'm offering to do it. If you're sure, why don't you 
let me
    >     > > > > > demonstrate, and prove myself wrong?
    >     > > > > >
    >     > > > > > >It would require lots of added API test coverage
    >     > > > > >
    >     > > > > > Require? That would be ideal, but we have supported minor 
versions
    >     > > for
    >     > > > > the
    >     > > > > > history of Traffic Ops, and never had extensive version 
tests. I
    >     > > agree we
    >     > > > > > should, but you're adding additional requirements to 
further your
    >     > > > > position,
    >     > > > > > which doesn't seem fair. Notwithstanding, the tag library 
already
    >     > > has 90%
    >     > > > > > test coverage and 3x as many lines of test code as logic; 
and the API
    >     > > > > Tests
    >     > > > > > are actually pretty easy, I just added one in the 
old-version-update
    >     > > fix,
    >     > > > > > and it was much easier than I expected:
    >     > > > > >
    >     > > > >
    >     > > 
https://github.com/apache/trafficcontrol/pull/3500/commits/16f2c96f086836f1d655fd62e673ee0a5e95e785
    >     > > > > > .
    >     > > > > >
    >     > > > > > >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
    >     > > > > >
    >     > > > > > I believe it is easy. The function to parse tags can use 
the tags in
    >     > > the
    >     > > > > > primary object (Cachegroup), and the sub-objects
    >     > > (LocalizationMethods)
    >     > > > > will
    >     > > > > > have their own version tags. I could be mistaken, I haven't 
actually
    >     > > > > > written the code yet, but I'm pretty sure sub-objects with
    >     > > sub-updates
    >     > > > > > won't be any more difficult or require much if any special 
logic.
    >     > > > > >
    >     > > > > >
    >     > > > > > On Thu, Apr 18, 2019 at 10:37 AM Gray, Jonathan <
    >     > > > > jonathan_g...@comcast.com>
    >     > > > > > wrote:
    >     > > > > >
    >     > > > > > > 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
    >     > > > > > >
    >     > > > > > >
    >     > > > > > >
    >     > > > >
    >     > >
    >
    >
    

  • ... Rawlin Peters
  • ... Fieck, Brennan
  • ... Robert Butts
  • ... Gray, Jonathan
  • ... Dan Kirkwood
  • ... Gray, Jonathan
  • ... Rawlin Peters
  • ... Dave Neuman
  • ... Jeremy Mitchell
  • ... Rawlin Peters
  • ... Gray, Jonathan
  • ... Fieck, Brennan
  • ... Robert Butts
  • ... Eric Friedrich -X (efriedri - TRITON UK BIDCO LIMITED c/o Alter Domus (UK) Limited -OBO at Cisco)
  • ... Rawlin Peters
  • ... Hank Beatty
  • ... Robert Butts
  • ... Hank Beatty
  • ... Rawlin Peters
  • ... Hank Beatty
  • ... Rawlin Peters

Reply via email to