| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-sdks - 2018-12-13 | |||
| 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. | |
| 16:35:41 | kmalloc | because it is useful. | |
| 16:35:52 | kmalloc | but that is a bigger bundle of "lots of comments" | |
| 16:35:57 | dtantsur | microversions are not contentious at all, just do them | |
| 16:36:05 | elmiko | dtantsur: LOL | |
| 16:36:14 | kmalloc | dtantsur: ok, go implement it for keystone, i think you just volunteered :) :P :P :P | |
| 16:36:20 | edleafe | kmalloc: http://specs.openstack.org/openstack/api-sig/guidelines/http/response-codes.html#failure-code-clarifications | |
| 16:36:35 | dtantsur | how many years it took us to implement proper negotiation in OSC? oh wait, it's still not there? :) | |
| 16:36:43 | edleafe | There is a paragraph there on 405 | |
| 16:36:50 | kmalloc | edleafe: yes, but i think that needs some expansion | |
| 16:37:05 | edleafe | patches welcome! :) | |
| 16:37:11 | kmalloc | yes, it's on my long long long list | |
| 16:37:14 | kmalloc | :) | |
| 16:37:21 | dtantsur | kmalloc: I'll switch keystone to SOAP | |
| 16:37:36 | kmalloc | dtantsur: if you can get people to approve it... and not break our API contracts, go for it | |
| 16:37:44 | kmalloc | dtantsur: i'll even volunteer to not -2 that change | |
| 16:37:50 | kmalloc | or even -1 it. | |
| 16:37:52 | edleafe | I think we should add a guideline that recommends abandoning the term "microversions", and replaces it with "every change is a major version" | |
| 16:37:52 | dtantsur | SOAP never greaks contracts, it's enterprise-grade | |
| 16:38:19 | dtantsur | edleafe: /me imagines $ curl https://baremetal.example.com/v56/nodes | |
| 16:38:56 | dtantsur | it's mostly kidding again, but I do feel bad that we break RESTfulness with microversions | |
| 16:40:06 | dtantsur | hehe | |
| 16:40:18 | elmiko | but then i am a smelly hippy, so there is that XD | |
| 16:40:23 | dtantsur | I got some funny experience with it | |
| 16:40:41 | elmiko | if you had "fun" anywhere near SOAP then you are doing better than most ;) | |
| 16:40:45 | dtantsur | It's supposed to be interoperable, but in fact I could not make a SOAP client in Java talk to a SOAP server in Python | |
| 16:40:53 | elmiko | ouch | |
| 16:40:54 | dtantsur | they had different SOAPs apparently | |
| 16:40:59 | elmiko | LOL | |
| 16:41:28 | edleafe | I made a python SOAP server talk with a .NET client | |
| 16:41:57 | dtantsur | nice! | |
| 16:42:34 | dtantsur | yeah, except for the problem "how to serialize a list". this one is unsolvable. something between "what's the point of life" and "where is my 2nd sock". | |
| 16:42:41 | cdent | dtantsur: which of many aspect of REST do we break with microversions. I suspect we could come up with several depending on which rules we wanted follow | |
| 16:43:25 | cdent | dtantsur: also I'm pleased to see you sticking to your guns with -1 the DELETE thing. I think it is important to get it not just right but agreed on and you raise some good points. | |
| 16:43:25 | dtantsur | cdent: the one where a URI points to a resource. with microversions a URL can mean anything (or nothing at all) depending on a header | |