Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2018-04-02
20:09:09 fried_bunny It needs to be described explicitly, because it's weird.
20:10:09 fried_bunny "Here's a new microversion where you have to send generation to ensure consistency. Oh, except the first time, when you have to send None (or leave it empty??). But if you do that when the consumer already exists, you'll fail.
20:10:11 fried_bunny "
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 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:12 openstackgerrit Eric Berglund proposed openstack/nova master: WIP: PowerVM: Cold Migrate & Resize https://review.openstack.org/553583
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

Earlier   Later