EDIT:

> I don't think our TO API version should be independent of the TC release 
> version

That was supposed to be:
"I think the TO API version should be independent of the TC release version"

On Fri, Apr 19, 2019 at 5:32 PM Rawlin Peters <[email protected]> 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
> <[email protected]> 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" <[email protected]> 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 <[email protected]> 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 <[email protected]> 
> > 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 <[email protected]> 
> > 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 <
> >     > > [email protected]>
> >     > > > 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 <[email protected]>
> >     > > > > Sent: Thursday, April 18, 2019 2:59 PM
> >     > > > > To: [email protected]
> >     > > > > 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 <[email protected]> 
> > 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 <
> >     > > > > [email protected]>
> >     > > > > > 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" 
> > <[email protected]>
> >     > > > > 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
> >     > > > > > >
> >     > > > > > >
> >     > > > > > >
> >     > > > >
> >     > >
> >
> >

Reply via email to