| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-sdks - 2018-12-13 | |||
| 16:20:21 | dtantsur | I don't know how to interpret it.. | |
| 16:20:25 | elmiko | edleafe: it just makes me wonder what the impl starts to look like, for example do i need to keep a database table of delete resources to properly respond to all future delete requests? | |
| 16:20:40 | dtantsur | edleafe: the "very single customer" is simply incorrect | |
| 16:20:40 | edleafe | elmiko: why? | |
| 16:20:53 | elmiko | if i delete some resource /foo/bar | |
| 16:21:09 | dtantsur | I personally very rarely catch 404 from deletions because I *want* to know that something went south | |
| 16:21:15 | elmiko | then later a request comes in to DELETE /foo/bar, then i need to respond 2XX if it has been deleted. is that accurate? | |
| 16:22:25 | edleafe | elmiko: if you call DELETE /foo/bar, and `foo` is a valid resouce class, you return 204 whether or not you had to delete resource `bar` or not. You don't need to confirm that `bar` ever existed. | |
| 16:23:09 | edleafe | dtantsur: that's very different than "I want this resource gone" | |
| 16:23:31 | edleafe | dtantsur: if you care that it's there and getting deleted, do a GET/HEAD on it first | |
| 16:23:33 | dtantsur | well, "I want this resource gone" is not the semantic I use, so no wonder :) | |
| 16:23:38 | elmiko | edleafe: what if "bar" never existed as a foo? | |
| 16:23:46 | elmiko | shouldn't that be a 404? | |
| 16:23:50 | edleafe | elmiko: No | |
| 16:23:58 | dtantsur | edleafe: so instead of handling 404 by some consumers, some other consumers have to make 2 requests? | |
| 16:24:03 | elmiko | ah, i misunderstood you then | |
| 16:24:13 | edleafe | if you interpret delete as "I don't want resource bar to exist" | |
| 16:24:28 | dtantsur | then we should allow 'DELETE /some/nonsense' :) | |
| 16:24:46 | dtantsur | half-kidding, but the same logic can be applied to invalid URLs | |
| 16:24:47 | edleafe | If you care about the existence of a resource, you call GET or HEAD | |
| 16:24:59 | kmalloc | it's fine to say DELETE X and if the path is unrouted, get a 404 | |
| 16:25:08 | kmalloc | afaict. | |
| 16:25:14 | kmalloc | no | |
| 16:25:14 | kmalloc | acutally | |
| 16:25:15 | dtantsur | kmalloc: why? I want it to be gone, and it's gone | |
| 16:25:18 | edleafe | dtantsur: cdent clarified that. Just what kmalloc said | |
| 16:25:20 | kmalloc | that would be a method not allowed | |
| 16:25:31 | dtantsur | edleafe: not for me | |
| 16:25:31 | kmalloc | sorry, wrong error | |
| 16:25:36 | kmalloc | not 404, 40.. 405? | |
| 16:25:42 | elmiko | kmalloc: if it's possible that the resource _could_ have existed, then it should 404. if i understand edleafe's point | |
| 16:25:48 | elmiko | er should not! | |
| 16:25:49 | elmiko | sorry | |
| 16:25:53 | kmalloc | elmiko: right | |
| 16:26:13 | kmalloc | elmiko: it should be .. 200/204/2XX (not 201) | |
| 16:26:18 | elmiko | right | |
| 16:26:28 | edleafe | Generally 204 is returned from DELETE | |
| 16:26:41 | kmalloc | if delete is not allowed, e.g. the whole path is unrouted in an api | |
| 16:26:44 | edleafe | There is no response body | |
| 16:26:54 | kmalloc | XXXX/<resource> -> where XXXX is not a thing at all | |
| 16:26:56 | kmalloc | that should be 405 | |
| 16:27:03 | kmalloc | as DELETE is not supported | |
| 16:27:18 | kmalloc | but if XXXX is a thing, and there could be a resource under it | |
| 16:27:19 | edleafe | kmalloc: according to cdent, it's 404 | |
| 16:27:20 | dtantsur | what about GET XXXX? | |
| 16:27:44 | kmalloc | edleafe: in most of openstack 404 is a thing. | |
| 16:27:47 | cdent | sorry, I'm debugging a gate failure | |
| 16:27:48 | kmalloc | most of openstack is wrong | |
| 16:27:59 | kmalloc | from a standpoint of the RFC | |
| 16:28:12 | edleafe | dtantsur: GET /XXXX should also be 404 | |
| 16:28:16 | cdent | only if XXXXX/<resource> has any methods at all should it ever be a 405 on delete | |
| 16:28:26 | cdent | if it simply does not exist then it should be 404 | |
| 16:28:35 | kmalloc | hm. | |
| 16:28:43 | kmalloc | i could see that interpretation | |
| 16:28:47 | dtantsur | kmalloc: well, we're talking of taking an RFC designed for hypertext and applying it to API | |
| 16:28:51 | kmalloc | order of precidence on routing | |
| 16:28:52 | cdent | kmalloc: that interpretation is right :) | |
| 16:28:53 | edleafe | From the front page of our guidelines: "Note Where this guidance is incomplete or ambiguous, refer to the HTTP RFCs—RFC 7230, RFC 7231, RFC 7232, RFC 7233, RFC 7234, and RFC 7235—as the ultimate authority." | |
| 16:28:54 | dtantsur | this has gone wrong many times, this is no exception | |
| 16:28:58 | cdent | the URI is the primary thing | |
| 16:29:01 | kmalloc | XXX is checked before XXXX/YYYY | |
| 16:29:05 | kmalloc | so, XXX hits 404 | |
| 16:29:09 | kmalloc | sure. | |
| 16:29:10 | cdent | if there is _nothing_ that has that URI, then 404 | |
| 16:29:19 | cdent | if you can GET it, but can't delete it, then 405 | |
| 16:29:54 | kmalloc | but a 404 on a delete where a resource could exist is explictly wrong | |
| 16:30:00 | kmalloc | it's always a success if delete could work | |
| 16:30:10 | kmalloc | cdent: ++ don't have to convince me too hard on that front :P | |
| 16:30:14 | cdent | that's effectively what ed's thing say | |
| 16:30:25 | edleafe | kmalloc: that's what the point of https://review.openstack.org/#/c/616610/ is | |
| 16:30:31 | edleafe | jinx | |
| 16:30:34 | kmalloc | hehe | |
| 16:30:40 | cdent | if this is a valid uri, and there might have been a thing here, or ever will be, respond 204 on delete when it is not there, or has just been removed | |
| 16:30:48 | kmalloc | yep. | |
| 16:30:51 | cdent | but if the url is invalid because you can't even GET there, then 404 | |
| 16:30:56 | cdent | s/even/ever/ | |
| 16:30:58 | kmalloc | yes. | |
| 16:31:07 | kmalloc | i could see that, i'll have to go poke and see how flask handles it | |
| 16:31:13 | kmalloc | i think it does it the correct way | |
| 16:31:20 | kmalloc | what you just described | |
| 16:31:31 | kmalloc | because if not, i have some bugs to go open/fix | |
| 16:31:38 | elmiko | edleafe: fwiw, i'm still +1 on the review | |
| 16:31:51 | elmiko | lol | |
| 16:31:59 | elmiko | seems like dtantsur will be the -1 | |
| 16:32:03 | dtantsur | yep | |
| 16:32:28 | edleafe | One thing to keep in mind before you -1: | |
| 16:32:45 | edleafe | This is a guideline; it says what projects should ideally follow | |
| 16:33:11 | edleafe | We are trying to encourage new projects to follow them. Existing projects do not have to change | |
| 16:34:08 | kmalloc | i know if we iterate on the keystone api like I want to, we'll get to where we're more closely following the guidelines | |
| 16:34:17 | kmalloc | we just have a ton of history that... well we can't "just fix" | |
| 16:34:29 | edleafe | kmalloc: sure, that's reasonable. | |
| 16:34:31 | elmiko | kmalloc: totally understandable | |
| 16:34:54 | elmiko | to echo edleafe, these are guidelines and not laws | |
| 16:34:59 | kmalloc | also we don't have (and doesn't look like we will in the near-to-medium term) microversions :) | |
| 16:35:08 | kmalloc | also +1 on that change. I like the guidelines | |
| 16:35:19 | edleafe | kmalloc: the other purpose of the guidelines is to "settle arguments". If a change is being considered, and there are differing views, the guidelines can be used as an arbiter | |
| 16:35:22 | elmiko | imo, microversions seem less contentious. but i'm just one person | |
| 16:35:33 | edleafe | elmiko: lol | |
| 16:35:34 | elmiko | edleafe ++ | |
| 16:35:36 | kmalloc | i'd like to really outline the use of 405 in that doc. | |