[GitHub] [trafficcontrol] ocket8888 commented on issue #3870: Rewrite /capabilities to Go

2019-08-15 Thread GitBox
ocket commented on issue #3870: Rewrite /capabilities to Go
URL: https://github.com/apache/trafficcontrol/pull/3870#issuecomment-521716358
 
 
   The best we could do in that direction is return an error without breaking 
the "API version promise". And actually, that might qualify; not sure.


This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [trafficcontrol] ocket8888 commented on issue #3870: Rewrite /capabilities to Go

2019-08-15 Thread GitBox
ocket commented on issue #3870: Rewrite /capabilities to Go
URL: https://github.com/apache/trafficcontrol/pull/3870#issuecomment-521713541
 
 
   Oh, rob05c beat me to it. But also of note while we're looking, the 
Role/Capability relationship has the same problem.


This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [trafficcontrol] ocket8888 commented on issue #3870: Rewrite /capabilities to Go

2019-08-15 Thread GitBox
ocket commented on issue #3870: Rewrite /capabilities to Go
URL: https://github.com/apache/trafficcontrol/pull/3870#issuecomment-521713196
 
 
   > _"If you update the name of a capability does it automatically update all 
the FK references in the api_capabilities table?"_
   
   It can, depends if the foreign key constraint was defined with `ON UPDATE 
CASCADE`. Which it should have been. But looking at 
`traffic_ops/app/db/create_tables.sql` it was not. Adding that clause is simple 
enough - and should be done, IMO -, but it's a database migration, so I'm not 
sure it's proper to include it in what's already a pretty expansive PR.
   
   Extensions certainly have access to the database at runtime, but idk if they 
do at startup.


This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [trafficcontrol] ocket8888 commented on issue #3870: Rewrite /capabilities to Go

2019-08-14 Thread GitBox
ocket commented on issue #3870: Rewrite /capabilities to Go
URL: https://github.com/apache/trafficcontrol/pull/3870#issuecomment-521515569
 
 
   1) `api_capability` links an API route to a particular capability.
   2) Yes and no. We need both _concepts_, but the routes governed by a 
capability could probably be dealt with under the `/capabilities` route itself.
   3) new capabilities can exist because arbitrary endpoints can be added via 
extensions. You can also group api routes under a one or more capabilities in 
arbitrary ways. Not sure that second one is a _good_ reason, but it is a true 
one.
   
   In any case, such deprecations probably need to go to the mailing list for 
discussion. I don't disagree that this could be handled better - possibly by 
requiring extensions to define a governing capability? - but I do want to just 
get the route into Go, since deprecation takes a full major release cycle 
anyway.


This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services