>What about a solution that still might employ nested, anonymous, versioned
structs, except that the only code that is unique to each versioned struct
is how requests of that version can be "upgraded" into requests of the next
latest version?
I'm +1 on that solution. I'm not 100% clear on what the code would look
like; something like `func (ds *DSV11) Upgrade() *DS`? I'd like to see the
PoC, but I don't see being opposed.
I'm not a fan of the db hit, but I'm willing to live with it, unless we see
evidence of it being a problem in the future.
> In this case, the only logic that would be unique to each minor version
would be:
> - how to upgrade requests of this minor version into the _next_ latest
minor version
> - how to downgrade results from the next latest minor version into _this_
minor version
I'm also hopeful this won't be too bad. We'll always have to have to
Sanitize/Validate even without versioning, hopefully this solution won't be
that much more than what we need anyway.
>super simple handlers for each previous minor version
We might even be able to do a little better than that, and only have one
handler, no separate versions in the routes/handlers. So, the route would
look something like:
func Update(w http.ResponseWriter, r *http.Request) {
inf, userErr, sysErr, errCode := api.NewInfo(r, nil, nil)
dsVer := tc.NewDeliveryService(inf.Version)
api.Parse(r.Body, inf.Tx.Tx, &ds)
ds := dsVer.Upgrade(inf.Tx.Tx)
And @rawlin.peters thanks for putting in the work to come up with this
idea. I know it takes work, to think of these things, and I for one
definitely appreciate it. It's definitely not any one person, I feel like
we have issues as a project working together and coming up with compromises
instead of hardlining, and I probably do it as much as anyone, and IMO we
all need to get better about it. So, thanks.
>I still prefer converting to PATCH instead of POST moving forward.
I'm a strong +1 on adding PATCH support to TO. I agree with @Jonathan_Gray
it's a huge win for safety, unnecessary network traffic, only changing the
fields that need changed, and fixing an otherwise nigh-unfixable Operations
race condition (and not just an Ops person doing the wrong thing, automated
scripts also race and overwrite each other).
But, we can never avoid PUT+POST entirely. There will always be cases where
POST is necessary, and places PUT makes a lot more sense and can actually
be safer than PATCH.
>You mean "PATCH instead of PUT"? I'd like to see us support both,
personally, but I still see the the data-loss thing as a separate issue.
They're separate, but related. PATCH mitigates a great many problems that
versioning solves. But again, we can't avoid POST+PUT entirely. But I
agree, probably better to separate that discussion into another thread.
On Tue, Apr 23, 2019 at 2:19 PM Fieck, Brennan <brennan_fi...@comcast.com>
wrote:
> > I still prefer converting to PATCH instead of POST moving forward
>
> You mean "PATCH instead of PUT"? I'd like to see us support both,
> personally, but I still see the the data-loss thing as a separate issue.
> There are lots of ways to handle that without worrying about versioning
> strategy.
>
> > We can't instantly
> upgrade every component of the CDN
>
> oh yeah...
> ________________________________________
> From: Rawlin Peters <rawlin.pet...@gmail.com>
> Sent: Tuesday, April 23, 2019 1:41 PM
> To: dev@trafficcontrol.apache.org
> Subject: Re: [EXTERNAL] Re: Traffic Ops API versioning issues
>
> On Tue, Apr 23, 2019 at 1:04 PM Gray, Jonathan
> <jonathan_g...@comcast.com> wrote:
> >
> > As a developer it sounds just as messy as the other solutions, but it
> may be workable. If you internally are chaining the object mashalling and
> db queries, how do you handle new mandatory fields that have no default? I
> still prefer converting to PATCH instead of POST moving forward.
> >
> > Jonathan G
>
> New mandatory fields with no default would not fall under the minor
> versioning strategy. Those would be breaking changes which would
> require a new major version. The new API endpoint under the next major
> version would essentially be a new unique struct and handler,
> implemented in such a way that it has no dependency on the previous
> major version. If we can't cleanly remove handlers and associated code
> for the previous major version (after a deprecation period) without
> affecting the new major version, then we've done something wrong.
>
> I think this solution would greatly organize the code. Part of the
> problem with the current implementation is that there is too much
> duplicated code between all of the different minor versions, and there
> is no clear boundary between what's expected of each minor version.
> For example, the delivery services API currently has multiple of the
> same functions attached to each minor-version struct, with the only
> real difference between each minor version being the struct itself for
> marshalling/unmarshalling purposes. The code already kind of
> "upgrades" requests (e.g. 1.1 to 1.5) to be handled by one "main"
> handler, except it's losing the 1.5 data that might already exist for
> that object by setting the 1.5 fields to their defaults. My proposal
> here would fix that data loss problem by reading the 1.5 fields into
> the struct _first_, before passing it up to the "real" handler to
> process the request as if it had been made at the latest minor
> version.
>
> What we would end up with would be super simple handlers for each
> previous minor version, with the latest minor version handler doing
> all of the legwork. The previous minor version PUT handlers might take
> a small hit in performance due to the need to read the newer fields
> from the DB (for the "upgrade" step), but as Traffic Control is
> primarily a read-heavy system, I think we could manage the small hit
> in latency. If the small increase in latency becomes a problem for you
> as a client of a previous minor version, all you have to do is upgrade
> your code to the latest minor version (which for the Go client should
> just be a recompile).
>
> - Rawlin
>
