Re: Deprecate /capabilities/{{name}}
I _could_ get on board with that schedule, but we can avoid unnecessarily breaking all users of the 1.x API in ATCv5 if we do this: ATCv3 has: api 1.x ~60% Go routes ~40% Perl routes ATCv4 has: api 1.x ~80% Go routes ~15% VALUABLE Perl routes (will be rewritten in Go) ~5% DEPRECATED Perl routes (e.g. /api_capabilities, /riak/stats, /cdns/configs/routing, etc) ATCv5 has: api 1.x 100% Go routes (remaining ~15% VALUABLE Perl 1.x routes have all been rewritten to Go, ~5% DEPRECATED Perl routes have been removed) At this point we have a fully-Go 1.x API with some crufty routes removed. We could possibly start overhauling 1.x to provide a 2.x API. The key point here is that whenever we remove the 1.x API, ALL users of the 1.x API will be broken and will have to fix their code to point at the 2.x API. Since the vast majority of 2.x routes would simply be promotions of Go routes from 1.x, this represents _unnecessary_ client breakage of ALL 1.x clients. By surgically deprecating crufty/should-never-be-used/unnecessary 1.x routes to be removed in a subsequent release, we'd only POSSIBLY be breaking SOME users that for whatever reason are using those crufty/unnecessary APIs. And to be clear, we're not talking about removing the bread-and-butter endpoints like /deliveryservices here, we're talking about things like /capabilities which users of the TO API have no reason whatsoever to be using in the first place. So it basically boils down to: A) promote Go 1.x routes to 2.x and eventually break ALL users of the 1.x API when the time comes to remove 1.x B) deprecate and remove some 1.x Perl routes and POSSIBLY break SOME users The difference between these two choices is just the decision of "strict, dogmatic adherence" to the "Semantic Versioning Promise" (which will cause ALL 1.x users to break eventually) or "relaxed adherence" (which will only POSSIBLY break SOME users that happen to be using useless API endpoints). - Rawlin On Tue, Aug 20, 2019 at 11:35 AM Jeremy Mitchell wrote: > > I can see both sides. The 1.x API contract should probably be respected but > I hate wasting time rewriting routes with little/no value. > > What about something like this? > > ATCv3 has: > api 1.x > ~60% Go routes > ~40% Perl routes > > *STOP all work on 1.x* > > ATCv4 has: > api 1.x > ~60% Go routes > ~40% Perl routes > api 2.x > 100% Go routes > - choose the ones we like from the 1.x's 60% and simply promote them to 2.x > - find the "valuable" perl ones that still need to be rewritten and rewrite > them to 2.x > > ATCv5 has: > api 2.x > api 3.x (at this point we can really overhaul the api and kill 1.x as well > (and the perl with it)) > > > This allows us 2 things: > 1. we respect the 1.x api contract > 2. we won't waste time rewriting perl routes of little/no use as we can be > selective what to put in 2.x > > It really feels like time to just move to 2.x to give us more flexibility, > otherwise, our hands are tied by the 1.x api contract. > > > > On Thu, Aug 15, 2019 at 2:34 PM Rawlin Peters > wrote: > > > Again, I disagree with you, yet we are on the same side here. I care > > about the community just the same as you do, albeit with somewhat > > different values in mind. I don't want to purposely break users by > > making unnecessary breaking changes to the API, but I also don't want > > to build and support an API that is full of unnecessary footguns. If a > > user ends up being broken by the removal of these seemingly useless > > capabilities endpoints, then I would question why they were even using > > them in the first place because they are not currently used anywhere > > in the codebase currently and have no effect whatsoever. IMO we are > > more likely to lose users due to having a more dangerous and > > unintuitive API than we are by removing endpoints that serve no > > purpose. > > > > - Rawlin > > > > > > On Thu, Aug 15, 2019 at 1:58 PM Robert Butts wrote: > > > > > > >We should be able to make exceptions to "the rules" in exceptional > > cases. > > > Deleting useless routes that provide no purpose to the system is what I > > > would consider an exceptional case. > > > > > > As above, I would consider an exception case where it's an unreasonably > > > huge amount of code and work, or danger. I do not consider it > > "exceptional" > > > for small, easy routes. > > > > > > > We should support API promises where they make sense and not get too > > > carried away with the "rules" where it doesn't make sense. > > > > > > -1. It does make sense here, it makes sense to maintain a stable project, > > > to attract users and committers. It's not about "following the rules," > > it's > > > about not breaking users. > > > > > > > Let's not let this thread devolve into API versioning. > > > > > > I agree, it should be a separate thread, if you want to raise this issue > > > again. But you can't state strong opinions that have huge impact (IMO > > > negatively) on the project, and then tell other people not to comment. >
Re: Deprecate /capabilities/{{name}}
I can see both sides. The 1.x API contract should probably be respected but I hate wasting time rewriting routes with little/no value. What about something like this? ATCv3 has: api 1.x ~60% Go routes ~40% Perl routes *STOP all work on 1.x* ATCv4 has: api 1.x ~60% Go routes ~40% Perl routes api 2.x 100% Go routes - choose the ones we like from the 1.x's 60% and simply promote them to 2.x - find the "valuable" perl ones that still need to be rewritten and rewrite them to 2.x ATCv5 has: api 2.x api 3.x (at this point we can really overhaul the api and kill 1.x as well (and the perl with it)) This allows us 2 things: 1. we respect the 1.x api contract 2. we won't waste time rewriting perl routes of little/no use as we can be selective what to put in 2.x It really feels like time to just move to 2.x to give us more flexibility, otherwise, our hands are tied by the 1.x api contract. On Thu, Aug 15, 2019 at 2:34 PM Rawlin Peters wrote: > Again, I disagree with you, yet we are on the same side here. I care > about the community just the same as you do, albeit with somewhat > different values in mind. I don't want to purposely break users by > making unnecessary breaking changes to the API, but I also don't want > to build and support an API that is full of unnecessary footguns. If a > user ends up being broken by the removal of these seemingly useless > capabilities endpoints, then I would question why they were even using > them in the first place because they are not currently used anywhere > in the codebase currently and have no effect whatsoever. IMO we are > more likely to lose users due to having a more dangerous and > unintuitive API than we are by removing endpoints that serve no > purpose. > > - Rawlin > > > On Thu, Aug 15, 2019 at 1:58 PM Robert Butts wrote: > > > > >We should be able to make exceptions to "the rules" in exceptional > cases. > > Deleting useless routes that provide no purpose to the system is what I > > would consider an exceptional case. > > > > As above, I would consider an exception case where it's an unreasonably > > huge amount of code and work, or danger. I do not consider it > "exceptional" > > for small, easy routes. > > > > > We should support API promises where they make sense and not get too > > carried away with the "rules" where it doesn't make sense. > > > > -1. It does make sense here, it makes sense to maintain a stable project, > > to attract users and committers. It's not about "following the rules," > it's > > about not breaking users. > > > > > Let's not let this thread devolve into API versioning. > > > > I agree, it should be a separate thread, if you want to raise this issue > > again. But you can't state strong opinions that have huge impact (IMO > > negatively) on the project, and then tell other people not to comment. > That > > isn't really fair to an open community. > > > > > > On Thu, Aug 15, 2019 at 1:51 PM Robert Butts wrote: > > > > > @mitchell852 How is that? > > > > https://cwiki.apache.org/confluence/display/TC/API+Guidelines#APIGuidelines-Deprecation > > > > > > > by that I mean not rewriting them to Go and changing the Perl routes > to > > > add a deprecation alert. Although, API promises and what-not aside, I > think > > > we could probably just delete the Perl routes right now because I can't > > > imagine a valid reason to even be using those endpoints currently. > > > > > > >I don't necessarily agree with rewriting deprecated Perl routes in > Go. I > > > think if a route is deprecated in release N, we can remove it in > release > > > N+1 (next major version). > > > > > > -1 > > > > > > >But if we're going to discuss this immediately, here's my two cents: > > > We've committed to API versioning. That means that you can't remove an > > > endpoint that existed in version 1.x without moving to 2.x. So not > > > rewriting all of the Perl endpoints under `/api/1.x/` would mean that > we're > > > committed to Perl sticking around until API 2.x. I think that's a bad > idea, > > > because the codebase gets much easier to work with when there's only > set of > > > source for a particular component. > > > > > > > You can't deprecate an endpoint in ATC version N and remove it in > N+1, > > > because the ATC version does not govern the TO API > > > > > > +1 > > > > > > The API version the the TC version are _completely separate things_. > Even > > > if the TC major version is increased, the API has its own "version > promise" > > > to avoid breaking clients. > > > > > > The problem here is not just breaking a TC CDN operator, it's also > > > breaking their clients. TC has an API, the whole point of an API is to > be > > > able to program against it reliably. > > > > > > The whole point of an "API Promise" is to allow clients, even if their > CDN > > > operator upgrades TO, to write scripts, which continue to work. Which > don't > > > break every few months, every time we delete some random route, > because we > > > decided we didn't think
Re: Deprecate /capabilities/{{name}}
Well, we've immediately derailed, lol. I just want to know if I can mark
`/capabilities/{{name}}` as deprecated.
But if we're going to discuss this immediately, here's my two cents: We've
committed to API versioning. That means that you can't remove an endpoint
that existed in version 1.x without moving to 2.x. So not rewriting all of
the Perl endpoints under `/api/1.x/` would mean that we're committed to
Perl sticking around until API 2.x. I think that's a bad idea, because the
codebase gets much easier to work with when there's only set of source for
a particular component.
You can't deprecate an endpoint in ATC version N and remove it in N+1,
because the ATC version does not govern the TO API (except insofar as I
believe introducing 2.x would necessitate bumping the ATC version).
Of course, we could always just _not_ version the API explicitly, which is
A-OK by me. But we've had that discussion before, and this particular email
thread isn't the place to re-hash it.
On Thu, Aug 15, 2019 at 1:16 PM Rawlin Peters
wrote:
> On Thu, Aug 15, 2019 at 1:01 PM Robert Butts wrote:
> >
> > >- don't bother converting `/capabilities/{{name}}` from Perl to Go
> >
> > But we still have to support those routes, until API 1.x is no longer
> > supported, which is far in the future.
> >
> > We should still rewrite deprecated Perl routes in Go, just so we can get
> > rid of Perl sooner than that.
>
> I don't necessarily agree with rewriting deprecated Perl routes in Go.
> I think if a route is deprecated in release N, we can remove it in
> release N+1 (next major version). We shouldn't be stuck maintaining
> useless endpoints for eternity (especially these ones which don't even
> make sense to support). If removing these deprecated endpoints before
> API 2.x is going to break any users out there, I would be glad to hear
> out their valid use cases for having us support them.
>
> Another example is the `cdns/:name/configs/routing` endpoint. It was a
> half-implemented attempt at breaking up the CRConfig and was not
> returning any valid or useful data at all when I last looked into it.
> Just because it exists in the Perl does not mean we should have to
> take it with us to Go. And the `cachegroup_fallbacks` endpoints, which
> are now supported in cachegroups proper. And the `/riak/stats`
> endpoint, which was mainly for triage purposes during the integration
> of Riak for Traffic Vault. We don't really have a good reason to keep
> those endpoints around IMO.
>
> - Rawlin
>
Re: Deprecate /capabilities/{{name}}
On Thu, Aug 15, 2019 at 1:01 PM Robert Butts wrote:
>
> >- don't bother converting `/capabilities/{{name}}` from Perl to Go
>
> But we still have to support those routes, until API 1.x is no longer
> supported, which is far in the future.
>
> We should still rewrite deprecated Perl routes in Go, just so we can get
> rid of Perl sooner than that.
I don't necessarily agree with rewriting deprecated Perl routes in Go.
I think if a route is deprecated in release N, we can remove it in
release N+1 (next major version). We shouldn't be stuck maintaining
useless endpoints for eternity (especially these ones which don't even
make sense to support). If removing these deprecated endpoints before
API 2.x is going to break any users out there, I would be glad to hear
out their valid use cases for having us support them.
Another example is the `cdns/:name/configs/routing` endpoint. It was a
half-implemented attempt at breaking up the CRConfig and was not
returning any valid or useful data at all when I last looked into it.
Just because it exists in the Perl does not mean we should have to
take it with us to Go. And the `cachegroup_fallbacks` endpoints, which
are now supported in cachegroups proper. And the `/riak/stats`
endpoint, which was mainly for triage purposes during the integration
of Riak for Traffic Vault. We don't really have a good reason to keep
those endpoints around IMO.
- Rawlin
Re: Deprecate /capabilities/{{name}}
TO-Perl currently has full CRUD API support for both /capabilities and
/api_capabilities. It seems to me like capabilities are
statically-defined in terms of what the actual capabilities of the
system are at the current version (via seeds.sql), so we don't really
need API-level support for Creating/Updating/Deleting capabilities.
That just seems like a footgun that we can easily eliminate from the
system altogether. I _do_ still think we should be able to READ the
existing capabilities via the API (for knowing what capabilities can
be added to a role), but that's about it.
If a TO extension adds a new route and wishes to use
roles/capabilities, it should insert its own capabilities into TODB at
startup/load time.
So, I would vote that we deprecate all Create/Update/Delete endpoints
for /capabilities and /api_capabilities, and by that I mean not
rewriting them to Go and changing the Perl routes to add a deprecation
alert. Although, API promises and what-not aside, I think we could
probably just delete the Perl routes right now because I can't imagine
a valid reason to even be using those endpoints currently.
- Rawlin
On Thu, Aug 15, 2019 at 12:15 PM ocket wrote:
>
> I recently marked a PR that rewrites the `/capabilities` endpoint in Go as
> ready for review (https://github.com/apache/trafficcontrol/pull/3870) and
> with it added handlers for PUT and DELETE. That means that
> `/capabilities/{{name}}` will no longer do anything that `/capabilities`
> does not, so is it alright if we deprecate it?
>
> There's also been some discussion about deprecating the ability to
> create/modify/delete capabilities at all (mostly on that PR itself, atm).
> I'd like to answer my above question before we get into that, though.
Re: Deprecate /capabilities/{{name}}
Maybe so I don't have to keep asking the question of what "deprecate" means
to us, we should add it to
https://cwiki.apache.org/confluence/display/TC/API+Guidelines so it's very
clear how we handle deprecated routes.
On Thu, Aug 15, 2019 at 12:54 PM Jeremy Mitchell
wrote:
> Can you elaborate on what "deprecate" actually means? Here's what i think
> it means:
>
> - don't bother converting `/capabilities/{{name}}` from Perl to Go and
> adding some kind of deprecation message to the `/capabilities/{{name}}`
> Perl response.
>
> On Thu, Aug 15, 2019 at 12:15 PM ocket wrote:
>
>> I recently marked a PR that rewrites the `/capabilities` endpoint in Go as
>> ready for review (https://github.com/apache/trafficcontrol/pull/3870) and
>> with it added handlers for PUT and DELETE. That means that
>> `/capabilities/{{name}}` will no longer do anything that `/capabilities`
>> does not, so is it alright if we deprecate it?
>>
>> There's also been some discussion about deprecating the ability to
>> create/modify/delete capabilities at all (mostly on that PR itself, atm).
>> I'd like to answer my above question before we get into that, though.
>>
>
Re: Deprecate /capabilities/{{name}}
>- don't bother converting `/capabilities/{{name}}` from Perl to Go
But we still have to support those routes, until API 1.x is no longer
supported, which is far in the future.
We should still rewrite deprecated Perl routes in Go, just so we can get
rid of Perl sooner than that.
On Thu, Aug 15, 2019 at 12:54 PM Jeremy Mitchell
wrote:
> Can you elaborate on what "deprecate" actually means? Here's what i think
> it means:
>
> - don't bother converting `/capabilities/{{name}}` from Perl to Go and
> adding some kind of deprecation message to the `/capabilities/{{name}}`
> Perl response.
>
> On Thu, Aug 15, 2019 at 12:15 PM ocket wrote:
>
> > I recently marked a PR that rewrites the `/capabilities` endpoint in Go
> as
> > ready for review (https://github.com/apache/trafficcontrol/pull/3870)
> and
> > with it added handlers for PUT and DELETE. That means that
> > `/capabilities/{{name}}` will no longer do anything that `/capabilities`
> > does not, so is it alright if we deprecate it?
> >
> > There's also been some discussion about deprecating the ability to
> > create/modify/delete capabilities at all (mostly on that PR itself, atm).
> > I'd like to answer my above question before we get into that, though.
> >
>
Re: Deprecate /capabilities/{{name}}
Can you elaborate on what "deprecate" actually means? Here's what i think
it means:
- don't bother converting `/capabilities/{{name}}` from Perl to Go and
adding some kind of deprecation message to the `/capabilities/{{name}}`
Perl response.
On Thu, Aug 15, 2019 at 12:15 PM ocket wrote:
> I recently marked a PR that rewrites the `/capabilities` endpoint in Go as
> ready for review (https://github.com/apache/trafficcontrol/pull/3870) and
> with it added handlers for PUT and DELETE. That means that
> `/capabilities/{{name}}` will no longer do anything that `/capabilities`
> does not, so is it alright if we deprecate it?
>
> There's also been some discussion about deprecating the ability to
> create/modify/delete capabilities at all (mostly on that PR itself, atm).
> I'd like to answer my above question before we get into that, though.
>
