| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2018-04-02 | |||
| 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 | |
| 21:37:58 | dansmith | er, wait wait, maybe I'm confused | |
| 21:38:07 | arvindn05 | dansmith: yup...i got that...can we discuss the comment? | |
| 21:39:18 | arvindn05 | http://specs.openstack.org/openstack/nova-specs/specs/rocky/approved/glance-image-traits.html#proposed-change | |
| 21:40:36 | arvindn05 | trait:CUSTOM_TRUSTED_HOST=required | |
| 21:40:36 | arvindn05 | trait:HW_CPU_X86_AVX2=required | |
| 21:40:36 | arvindn05 | we are adding attributes like below to the image dict to support filtering based on traits expressed in images | |
| 21:40:53 | dansmith | arvindn05: yeah I know that part | |
| 21:42:22 | dansmith | okay, sorry I had this all tangled in my head | |
| 21:42:48 | dansmith | I was thinking this was the path in for the legacy dict, but this is actually the path in for image meta itself, which calls the name mapping stuff inside here | |
| 21:42:52 | arvindn05 | no problem...sorry i just threw it over the wall | |
| 21:45:45 | arvindn05 | yes...i think. the methods parses all the properties expressed in the image meta data(as a key:value strings) both legacy and non-legacy and creates the ImageMetaProps out of it | |
| 21:46:24 | dansmith | arvindn05: replying to try to clean up my mess, hang on | |
| 21:52:03 | dansmith | arvindn05: see if that makes sense | |
| 21:52:27 | arvindn05 | thx..looking at it now | |
| 21:52:28 | dansmith | something he said about none-ness sent me down the wrong path in thinking what this was about | |
| 21:55:23 | arvindn05 | got it...makes sense | |
| 21:55:56 | arvindn05 | dansmith: thanks...will make the update | |