| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2018-04-02 | |||
| 20:11:46 | edleafe | it might not be in the spec | |
| 20:12:23 | edleafe | But it's not that weird. Standard practice is to get existing allocations before adding any new ones | |
| 20:12:49 | edleafe | I know it was discussed | |
| 20:12:52 | fried_bunny | okay, so what will you return? | |
| 20:12:55 | fried_bunny | in that first one? | |
| 20:13:00 | fried_bunny | Right now you get {} | |
| 20:13:04 | edleafe | but those discussions happened in several places at various times | |
| 20:13:12 | fried_bunny | Will you return { "consumer_generation": None } ? | |
| 20:13:43 | fried_bunny | or am I responsible for doing result.get('consumer_generation', None) ? | |
| 20:13:44 | edleafe | If there are no allocations for a consumer, then there is no consumer | |
| 20:14:19 | fried_bunny | Cool. So when I do my PUT, do I say "consumer_generation": None, or do I exclude the "consumer_generation" field entirely? | |
| 20:14:50 | edleafe | If you're PUTting to the new microversion, you would have to pass the generation, so the former | |
| 20:15:10 | fried_bunny | In which case the schema needs to tolerate [int or null] | |
| 20:15:29 | edleafe | guess it will have to :) | |
| 20:16:04 | edleafe | I'm just starting the patch that adds the microversion. I think these cases will make for excellent functional tests | |
| 20:16:24 | fried_bunny | I'm gonna mark up the spec review. | |
| 20:16:58 | edleafe | good idea | |
| 20:18:59 | openstackgerrit | Eric Berglund proposed openstack/nova master: PowerVM Driver: Localdisk https://review.openstack.org/549300 | |
| 20:21:12 | openstackgerrit | Eric Berglund proposed openstack/nova master: WIP: PowerVM: Cold Migrate & Resize https://review.openstack.org/553583 | |
| 20:21:12 | melwitt | fried_bunny: replied to your comments. this is my remaining open question https://review.openstack.org/#/c/533821/25/nova/tests/functional/api/openstack/placement/test_report_client.py@812 | |
| 20:21:27 | fried_bunny | melwitt: Roger that, looking... | |
| 20:21:35 | fried_bunny | (thought you were bunnying today, melwitt) | |
| 20:22:02 | melwitt | unfortunately no | |
| 20:26:09 | fried_bunny | edleafe: https://review.openstack.org/#/c/556971/ done been marked up. | |
| 20:31:24 | fried_bunny | melwitt: Responded. | |
| 20:37:41 | fried_bunny | edleafe: tbc, you're not intending to do the microversion bump in https://review.openstack.org/#/c/557959/ right? | |
| 20:38:26 | edleafe | fried_bunny: yeah, it will be the next one in that series | |
| 20:38:41 | fried_bunny | rgr | |
| 20:39:54 | openstackgerrit | Eric Berglund proposed openstack/nova master: PowerVM Driver: Localdisk https://review.openstack.org/549300 | |
| 20:41:29 | openstackgerrit | Eric Berglund proposed openstack/nova master: WIP: PowerVM: Cold Migrate & Resize https://review.openstack.org/553583 | |
| 20:45:54 | fried_bunny | edleafe: Hum, we need to change the POST /allocations and PUT /allocations/{c} responses to 200 with payload. | |
| 20:46:03 | fried_bunny | they're 204 right now. | |
| 20:49:00 | fried_bunny | starting to look like a mriedem commentary - three separate comment-leavings in a row on the same patch. | |
| 20:56:13 | edleafe | fried_bunny: not sure I get the reasoning behind changing the 204 from PUT/POST | |
| 20:57:08 | fried_bunny | edleafe: It's the same reason we did http://specs.openstack.org/openstack/nova-specs/specs/rocky/approved/generation-from-create-provider.html | |
| 20:58:21 | fried_bunny | edleafe: So clients don't have to do GET, PUT, GET, PUT -- can just do PUT, PUT instead. | |
| 20:58:37 | edleafe | But for a consumer? RPs are long-lived and frequently changed. Consumers rarely get changed, and I can't think of a use case where you would create allocs, and then create some more right away | |
| 21:01:28 | fried_bunny | eh? | |
| 21:01:42 | fried_bunny | Hold on, processing that... | |
| 21:01:43 | melwitt | dansmith: is this your comment on L45? https://etherpad.openstack.org/p/nova-runways-rocky if all non-WIP changes are merged, I think we can bump the blueprint out of the runway and put the next in line in the runway. does that sound cool? | |
| 21:02:08 | dansmith | melwitt: bope not mine | |
| 21:02:10 | dansmith | er, nope | |
| 21:02:32 | fried_bunny | edleafe: But instinctive reaction is: We need to avoid focusing on Nova as the only consumer (again/still). | |
| 21:02:33 | dansmith | melwitt: but yes, the WIP bits are waiting on pieces of placement that don't exist yet, so it's "done" | |
| 21:02:56 | melwitt | oh, guh. I'm really bad at remembering what author color people are | |
| 21:03:08 | fried_bunny | it changes, too :( | |
| 21:03:11 | dansmith | melwitt: maybe want to, at least initially, copy things out of a runway to a log area and put some comments about how it went? | |
| 21:03:19 | edleafe | fried_bunny: I'm not thinking of Nova | |
| 21:03:35 | melwitt | dansmith: sounds like a good idea | |
| 21:03:36 | dansmith | melwitt: for that set, very little review was had for about a month and then it got review, iteration, and then merged quickly whilst in that slot | |
| 21:03:41 | fried_bunny | dansmith, melwitt: ++ to that | |
| 21:03:52 | fried_bunny | (the keeping-logs-of-runways) | |
| 21:04:10 | edleafe | for any placement user: if you are making allocations, you would GET the current state, modify those allocations to add/remove what is needed, and then PUT that back | |
| 21:04:36 | fried_bunny | edleafe: And cache that, so that next time, you don't have to reGET. | |
| 21:04:46 | fried_bunny | exactly like we're doing for RPs. | |
| 21:05:01 | edleafe | fried_bunny: you'll have to come up with a scenario where that is at all plausible | |
| 21:05:06 | fried_bunny | resize | |
| 21:05:10 | fried_bunny | and | |
| 21:05:14 | fried_bunny | symmetry with the RP API | |
| 21:05:16 | fried_bunny | :P | |
| 21:05:45 | melwitt | jackie-truong: hi, your blueprint https://blueprints.launchpad.net/nova/+spec/nova-validate-certificates is next in line for a review runway that we're looking to fill. is now a good time to iterate quickly on code review for the next two weeks in a runway? | |
| 21:05:47 | fried_bunny | Is it possible to GET before PUT every time? Sure. But so easy to avoid, so why? | |
| 21:05:59 | edleafe | with resize you'd only change the consumer once | |
| 21:06:16 | edleafe | Symmetry is only important if the things are similar enough | |
| 21:06:20 | fried_bunny | edleafe: per resize | |
| 21:06:45 | edleafe | ?? | |
| 21:06:50 | fried_bunny | edleafe: or maybe not. Maybe we're passing the allocation around to cinder to change storage allocs, cyborg to change dev allocs, neutron to change net allocs.... | |
| 21:07:13 | fried_bunny | So it's half a dozen times per spawn, per resize, per whatever. | |
| 21:07:38 | edleafe | fried_bunny: that's the case for adding a generation. These will be presumably done in parallel, so it is racy by desing | |
| 21:07:42 | edleafe | design, even | |
| 21:07:46 | fried_bunny | But what's the resistance? Just that it's more code? | |
| 21:08:08 | edleafe | no, because it doesn't fit how allocations should work | |
| 21:08:19 | fried_bunny | well, that I disagree with. | |
| 21:09:07 | fried_bunny | If you had said it's because we don't have an immediate use case for reusing the allocation, I would still argue it's easier to do it now than in a new microversion later. | |
| 21:10:28 | edleafe | Let's put this on hold and bring in jaypipes and cdent. One of us is not seeing things correctly | |
| 21:11:16 | fried_bunny | rgr | |
| 21:11:25 | fried_bunny | I commented in the spec. We can discuss there. | |
| 21:15:34 | jackie-truong | melwitt: Yes, we can be available for the next two weeks to respond to code reviews | |
| 21:16:16 | melwitt | jackie-truong: sweet, I shall add the blueprint to the runway then. thanks | |
| 21:17:53 | jackie-truong | melwitt: Thanks! | |
| 21:18:09 | arvindn05 | are there any guidelines for initialization of datastructures in nova specifically? I have an object with a dictionary which may or may not be populated... | |
| 21:18:44 | arvindn05 | should we always initialize instead of making it None? | |
| 21:20:36 | dansmith | arvindn05: that's probably too vague to answer without seeing what you're talking about | |
| 21:20:49 | arvindn05 | https://review.openstack.org/#/c/557795/ | |
| 21:21:22 | arvindn05 | sorry...here is the review for context. https://review.openstack.org/#/c/557795/7/nova/objects/image_meta.py | |
| 21:22:41 | arvindn05 | i was trying to follow the pattern within the file where complex fields like ListOfSetsOfIntegersField are not initialized if there are no properties | |
| 21:25:01 | dansmith | arvindn05: I commented | |
| 21:25:46 | dansmith | arvindn05: you should look at other patches to that object in recent history and see the pattern they use.. it doesn't matter that this is a "complex field" | |
| 21:27:29 | arvindn05 | ok...but was wondering if there were guideline documented somewhere on passing None vs initialized objects... | |
| 21:27:53 | dansmith | arvindn05: as I commented there, it makes no sense to be concerned that someone would pass None there | |
| 21:29:08 | arvindn05 | so you propose removing the if? I think the if is guarding against both empty properties as well as None | |
| 21:30:15 | dansmith | arvindn05: did you read my comment? | |
| 21:30:22 | dansmith | I'm proposing you remove the entire function | |
| 21:30:50 | arvindn05 | yup getting to that :) | |
| 21:31:21 | arvindn05 | Also on the second comment, The method is to initialize/create the image meta props object from the dict | |
| 21:32:13 | arvindn05 | we are adding new attributes in the dict as per the blueprint https://blueprints.launchpad.net/openstack/?searchtext=glance-image-traits | |
| 21:33:06 | dansmith | arvindn05: none of the other methods called before yours handle that parameter being None, so it will never get to your if condition if the thing is None | |
| 21:35:58 | arvindn05 | yup...but they do handle empty dict...in case of an empty dict, they do not initialize the fields | |
| 21:36:14 | arvindn05 | the if condition there is handling the empty dict case | |
| 21:37:29 | dansmith | arvindn05: none of this matters since you shouldn't be setting that from from_dict() anyway, per my comment | |