Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2018-01-30
22:17:28 edleafe efried: That's what DELETE is designed to do.
22:17:52 edleafe efried: No, PUT {} is an awkward way of saying DELETE
22:18:49 efried edleafe: Yeah, I get it. Is it "wrong" (in the annals of HTTP, or REST, or APIs, or whatever) for a DELETE API to return a payload?
22:20:30 sean-k-mooney efried: yes i belive it is not ment to have a payload generally
22:20:46 edleafe efried: Unless the response is 204.
22:21:01 mmedvede sean-k-mooney: am I missing something? Both http://ci-watch.tintri.com/project?project=nova and https://dal05.objectstorage.softlayer.net/v1/AUTH_3d8e6ecb-f597-448c-8ec2-164e9f710dd6/pkvmci/index.html do not indicate a systemic failure on nova patches
22:21:09 mmedvede for powerkvm ci
22:21:40 sean-k-mooney mmedvede: there was a message a week or two saying it was i was not sure if it was fixed or not
22:21:56 edleafe efried: Othewise, you can return either a description of the deleted resource (200) or a URL to check for success/failure on a 202.
22:22:28 efried edleafe: So it'd be acceptable for a DELETE to return 200 with a payload?
22:23:01 edleafe efried: it's required
22:23:07 mriedem sean-k-mooney: are you thinking of the zvm ci?
22:23:23 mriedem or zkvm i mean
22:23:58 efried edleafe: Sorry, I mean I get that it's cool for DELETE to respond 204 with no content; I'm asking whether there's any restriction - standard-wise or cultural - against a DELETE responding 200 with a payload.
22:24:07 mriedem mmedvede: i had seen some other pkvm ci failures in stable branches, but those might have been old/transient
22:24:11 efried and I think you've said that's acceptable.
22:24:46 edleafe efried: yes, it's acceptable, although it isn't very common
22:26:14 sean-k-mooney mriedem: yes i was https://www.mail-archive.com/openstack-dev@lists.openstack.org/msg115082.html
22:27:29 mriedem sean-k-mooney: mixing up one of the dozen ibm 3rd party CIs is grounds for pistols at dawn
22:28:02 mmedvede mriedem: yes, stable branches have high rate of failure unfortunately, I'll shift some time to look at those.
22:28:53 sean-k-mooney mriedem: haha well the grenade job is failing because of a ubuntu keyring missing on the powervm ci too but that could be intermitent
22:29:33 mmedvede sean-k-mooney: that is intermittent, there is a bug in ubuntu somewhere that we reported
22:29:59 mmedvede it happens only last 10 minutes of any hour
22:30:21 sean-k-mooney mmedvede: ya the patch i noticed it on is for rocky anyway so im not going to waste ci time rechecking
22:32:47 sean-k-mooney anyway i have fixed my unrelated ovs db socket somehow became a directory and broke everything issue with kolla so im going to head home for the evening
22:32:58 efried edleafe: btw, in case it wasn't obvious, this is pursuant to what we were discussing the other day. Without a response payload, we have to assume things about the effect of DELETE on provider generation. One possible solution is to use PUT <empty> where available, which it happens to be for all of these. Another is to implement DELETE with a response payload from which we can glean the new generation.
22:36:16 edleafe efried: Aren't we sending the generation along with the PUT/DELETE request?
22:37:00 efried edleafe: With PUT, yes. Not with DELETE, which doesn't accept a payload. The latter is a definite (but separate) problem.
22:37:37 efried edleafe: But even the former only guarantees that we're deleting what we thought we were deleting. The lack of generation in the return is a problem for *subsequent* updates.
22:38:11 efried ...unless we continue to make assumptions about how placement does generations. Which IMO is wrong.
22:38:40 edleafe efried: So say I get the generation back from the PUT/DELETE. Right after that, other requests modify the resource. What good does getting back gen+1 from the request do me then?
22:39:54 efried edleafe: In that scenario, it doesn't save you anything, because your next update will 409 and you have to re-GET the provider and its associated stuff before you redrive your update.
22:40:19 efried edleafe: It's in the scenario where the resource provider *doesn't* change that it saves us.
22:40:36 edleafe efried: I still don't see that
22:40:38 efried edleafe: Because without it, we *have* to do that re-GET in order to glean the new generation.
22:40:46 efried (again, unless assumptions)
22:41:00 edleafe efried: Oh, you're still pretending that we don't know how generations work
22:41:04 efried Correct.
22:41:08 edleafe The name itself should be a giveaway
22:41:21 efried say wha?
22:41:33 efried Please tell me you're joking
22:41:40 edleafe no, I'm not
22:42:14 efried We're not going to assume API behavior based on the dictionary definition of an ambiguous English word with lots of synonyms.
22:42:35 efried even if any one of those definitions said "and we start at 0, and the next one is +1" - which they don't.
22:43:19 edleafe We adopted generations over other candidates, such as etags, precisely because we *could* make these assumptions
22:43:36 edleafe I understand that we have not documented that behavior well enough for outsiders
22:43:47 efried Or insiders, apparently. I think I qualify.
22:43:51 edleafe But within the placement dev team, it was clear
22:44:06 edleafe were you in the discussions about what to use?
22:44:32 efried Oh, by "outsiders" did you mean whoever wasn't in the design discussion?
22:44:40 edleafe yes
22:44:51 efried I thought you meant people who aren't developers of the placement API.
22:45:15 edleafe well, you have seemed to pick up on the way it works :_
22:45:50 efried Yes, and I vociferously contend that we have not documented that behavior well enough.
22:46:04 edleafe and I will vociferously agree with that
22:46:14 efried In any case, based on last week's straw poll, I don't think there's consensus even among people who were in the room when it was designed.
22:46:32 efried ...as to whether the behavior should be an explicit, documented part of the API.
22:46:34 edleafe some of us weren't happy with the choice :(
22:46:38 mriedem you guys are talking about a decision made a couple of years ago right?
22:46:50 mriedem in a kind of 'shit or get off the pot' situation?
22:46:54 edleafe mriedem: well, about a year or so I think
22:47:04 mriedem not knowing what we'd be doing with these things several releases later
22:47:19 efried mriedem: We're not even arguing the decision to implement generation a certain way. I don't think anyone has a problem with that.
22:47:26 edleafe there was the opaque marker (etag) proponents, and the deterministic marker (generation) camp
22:47:28 mriedem a year would have been the ATL PTG and it seems older than that, at least BCN
22:48:07 mriedem so next step is....documenting how generations work
22:48:08 mriedem ?
22:48:29 efried mriedem: Yeah - what's at issue is whether that implementation is supposed to be known by consumers of the API, or whether it's supposed to be opaque.
22:49:13 mriedem sorry, i haven't followed this debate the last several times it's come up, but what's the issue? that a client can get a generation that skips increments?
22:49:21 mriedem like t1 i get gen 1, t2 i get gen 3?
22:50:22 efried Not really. The issue is, can the consumer count on a) generation starts at 0 when you create a new provider; and b) generation increments by 1 when you successfully update something associated with a provider.
22:51:06 efried Point being, as currently implemented, yes they can, but do we want that to be explicitly part of the API, or do we want it to be opaque and you shouldn't introspect the generation?
22:52:14 edleafe mriedem: IOW, do we want to require that each call to modify a RP is first preceded by a GET to find the current generation?
22:52:24 efried no, not at all.
22:52:34 efried that would not be necessary at all.
22:52:59 mriedem if the generation the caller passes is in conflict, they get a 409, get the latest generation, merge the request and try again
22:53:17 mriedem why would the caller need to know that the generation increments by 1?
22:53:29 edleafe efried: how can they pass a generation if they can't assume previous+1?
22:54:01 mriedem efried: fwiw i think "can the consumer count on a) generation starts at 0 when you create a new provider; and b) generation increments by 1 when you successfully update something associated with a provider." is fine
22:54:04 efried edleafe: Because the response from any update includes the new generation. Since the update was atomic, you know that the payload in the response corresponds to that generation.
22:54:41 efried And I can keep my cache consistent with that information.
22:55:38 efried And the problem arises any time there's an update from whose response I can't glean the new generation. Like DELETE inventory, allocations, traits (and someday maybe aggregates)
22:56:08 efried mriedem: Duly noted. I think your vote splits us clean down the middle.
22:56:59 mriedem consider it a very limp vote
22:57:09 efried If I'm remembering right, we have mriedem cdent edleafe on one side, and efried dansmith jaypipes on the other.
22:57:44 mriedem well, i also agree that "you shouldn't introspect the generation"
22:57:48 edleafe and IIRC, it was jaypipes advocating for the gen+1 back in the day
22:58:03 mriedem like i said above, as a client, try the request, if it fails, fetch the latest thing, merge the result client side and try again
22:58:16 edleafe for the very reason that you *could* assume those things
22:58:28 efried edleafe: Not to put words in his mouth, but he may have been advocating the monotonic integer implementation without advocating that that implementation be transparent to the API consumer.
22:58:54 edleafe efried: that would have no advantage over something like etag
22:59:01 mriedem i sure hope this is blocking the most important thing that anyone is ever going to be working on...because it sures feels like it
22:59:09 edleafe it was that advantage that was the main argument
23:00:15 edleafe mriedem: it's just that we went through this a while ago, and now it's being dragged up again for the very reason it was chosen in the first place
23:01:55 mriedem fair
23:01:57 mriedem but,
23:02:02 mriedem it's 5:02 and i'm out of here suckas
23:02:08 efried later mriedem
23:02:15 openstackgerrit Merged openstack/nova master: Cleanup redundant want_version assignment. https://review.openstack.org/535801
23:02:25 openstackgerrit Merged openstack/nova master: Add debug output for selected page size https://review.openstack.org/530662

Earlier   Later