[GitHub] [trafficcontrol] rob05c commented on issue #3966: Add server capabilities API
rob05c commented on issue #3966: Add server capabilities API URL: https://github.com/apache/trafficcontrol/pull/3966#issuecomment-540889517 > 1. message here needs to be corrected. we are deleting server_capabilities only by name. Yep, looks like an artifact of `api.GenericDelete`. Good catch! Looks like either `api.GenericDelete` will have to be changed to something like "no x with that key found," or otherwise extended to know the names of the key(s). > 2. when spaces are at the start and end of name, they are trimmed and accepted instead of throwing error. That should be fine. It should be documented, but the extra safety seems like a good thing to me. > 3. api parameters are not validated. for instance, requesting service_capabilities endpoint with param id doesn't throw any error. The API shouldn't do this. Unknown parameters should be ignored. This follows the Robustness Principle. See https://tools.ietf.org/html/rfc1122#section-1.2.2 https://en.wikipedia.org/wiki/Robustness_principle This makes our API easier to use, and less likely to break users, and more forward-compatible. For example, someone could be using a client with a newer version, or newer code within the same version, but knowingly double-checking their filtering params (atstccfg does this); or posting some kind of debug information that helps themselves; or simply making a simple mistake that doesn't need to break things. I know some other APIs do this. It is a gross violation of the Robustness Principle, and I strongly disapprove. It makes servers brittle and fragile, and complicates integration and forward-compatibility. 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] rob05c commented on issue #3966: Add server capabilities API
rob05c commented on issue #3966: Add server capabilities API URL: https://github.com/apache/trafficcontrol/pull/3966#issuecomment-540240343 > Should POST/DELETE only be admin? Right now I have it set to Operations. I think Ops is right. Admin vs Ops is somewhat nebulous, but I think in general, Operations can do pretty much everything except see password and security stuff. So I think Operations is right for these endpoints. 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] rob05c commented on issue #3966: Add server capabilities API
rob05c commented on issue #3966: Add server capabilities API URL: https://github.com/apache/trafficcontrol/pull/3966#issuecomment-540057546 >without reading thru the code, does DELETE validate that the "server capability" is: >a. not being used by any server and >b. is not required by any ds Server-Capabilities and DS-Required-Capabilities don't exist in this PR, that'll have to be added in a subsequent PR. But, I'd vote we don't add custom Go logic to handle that - rather, if we make them proper Foreign Keys that don't CASCADE DELETE, the DB will automatically prevent it. Then, all we have to do is parse the Postgres error to return a useful message to the user (we already have a func for that `api.ParseDBError` 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
