Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2018-03-26
15:21:06 gibi jaypipes: replied in https://review.openstack.org/#/c/502306 but I have to go offline in 5. I will check your reply tomorrow morning
15:21:30 mriedem stephenfin: https://review.openstack.org/#/c/534724 drops a lot of info
15:22:05 stephenfin mriedem: Fair points. I'll address those
15:22:11 stephenfin Thanks for the review (y)
15:23:55 gibi Kevin_Zheng: I left some suggestion in https://review.openstack.org/#/c/553288
15:25:36 mriedem tssurya: figured out why you can't run nova-api without having [database]/connection setup http://logs.openstack.org/46/555346/2/check/tempest-full/69cf0dc/controller/logs/screen-n-api.txt.gz#_Mar_24_00_53_19_452955
15:25:51 mriedem tssurya: File "/opt/stack/nova/nova/api/openstack/wsgi_app.py", line 49, in _setup_service is not multi-cell aware
15:25:59 mriedem belmorei_: ^
15:26:53 mriedem that code should likely just lookup the cell0 mapping and use it's context
15:27:09 tssurya mriedem: ah okay
15:27:51 kashyap dansmith: Responded; I'm not quite sure if a soft log warning would suffice...
15:28:15 belmorei_ mriedem thanks
15:28:55 openstackgerrit Eric Fried proposed openstack/nova master: Unit test framework: common FakeResponse https://review.openstack.org/556551
15:29:01 efried mriedem: Done. More red! Yay!
15:29:09 kashyap dansmith: Also, about verbosity in the `reno`, I was aware of it; but I was not merely describing _what_ are those 3 modes.
15:29:20 tssurya mriedem: thanks for spotting it
15:29:35 kashyap dansmith: Rather, what would Operators want to do in context of the three modes Nova allows.
15:29:46 kashyap (As I've lost count on other Virt lists & IRC where people have asked about it.)
15:30:41 kashyap That said ... happy to snip it, and add it to a separate blog post or something. I'm all for brevity with clarity.
15:31:01 dansmith kashyap: ask someone else, but IMHO, it's about 500% too wordy
15:31:22 kashyap dansmith: Folks on #openstack-release said it reads very well, FWIW. smcginnis and dhellmann reviewed it
15:31:46 dansmith kashyap: cool, but I think it's too much
15:32:23 kashyap dansmith: I can cut it down. But I'd rather want to definitely retain the info about what an Operator would do with each of the 3 modes
15:32:34 kashyap It _certainly_ makes sense. As it's a completely valid question that will come up.
15:32:43 kashyap (As not everyone dwells on it.)
15:32:54 dansmith kashyap: that's fine, but I'll still be -1 on it
15:33:43 kashyap I'd rather first get someone else's view too in Nova.
15:34:36 dansmith kashyap: didn't I say ask someone else? :)
15:34:52 kashyap Sure :-)
15:35:46 dansmith efried: are you looking for me to add aggregates to ResourceRequest, or just to slap it into the result of resources_from_request_spec? because it seems like that function does some weirdness where it requests some resource group and then jams extra resources into the result
15:36:06 dansmith efried: this specifically: https://github.com/openstack/nova/blob/master/nova/scheduler/utils.py#L334
15:36:58 dansmith I guess it's returning some internal object and then jamming the resource request in there
15:37:02 efried dansmith: That bit is for special handling of the "default resource classes". Wouldn't think aggregates would play in there.
15:37:19 dansmith efried: yeah, I'm trying to figure out where aggregates go and reading that is confusing to me
15:37:33 efried dansmith: Anyway, yeah, I was looking for ResourceRequest to get an agg field (similar to the traits field) which would be populated based on the request spec.
15:37:37 dansmith efried: you want an setter method on ResourceRequest that takes aggregates?
15:37:38 efried ...using the code you already done wrote.
15:37:50 dansmith or can I just set res_req.aggregates = []
15:37:52 dansmith ?
15:38:30 dansmith like, resources_from_request_spec() seems like it should be a classmethod on ResourceRequest, but it's not so I'm trying to figure out how much of "friends" they are
15:38:52 efried dansmith: um, looks like I already have member_of on RequestGroup .
15:39:13 dansmith okay and I don't really get the RequestGroup thing
15:39:24 dansmith and that comes out of placement,
15:40:08 dansmith so in that case, I _would_ actually do aggregates like the extra resources are getting jammed in there?
15:40:18 dansmith res_req.get_request_group(None).member_of = aggregates ?
15:40:25 efried yeah
15:40:33 dansmith mkay
15:40:42 dansmith I don't really get what this is doing, but.. as you wish
15:40:52 efried dansmith: It's the framework for granular.
15:41:04 dansmith ah, and that's what ident is then.. okay
15:41:37 efried dansmith: Puts in one place the parsing of the extra specs into the RequestGroups which will feed into the placement call.
15:42:12 dansmith efried: so, RequestGroup doesn't have much schema, so do you want me to:
15:42:21 dansmith or
15:42:21 dansmith grp.member_of = ['foo,bar', 'baz']
15:42:37 dansmith grp.member_of = [('foo','bar'), ('baz',)]
15:42:38 dansmith ?
15:42:57 efried dansmith: Guess it depends on how that spec shakes out :P
15:43:02 jaypipes gibi: hey, sorry, went to get something to eat. I'll respond on the spec.
15:43:16 openstackgerrit Matthew Booth proposed openstack/nova-specs master: Add serial numbers for local disks https://review.openstack.org/556565
15:43:26 dansmith efried: I don't think it really does, this is just the internal way we communicate the request to report client right?
15:43:49 dansmith efried: I'm not really sure why this lives in placement.lib I mean
15:43:50 efried dansmith: yeah. I think the sooner we get to the list-of-tuples representation, the better. So option 2
15:43:50 dansmith because later that will be out of tree and we won't use it to communicate with our own reportclient I think
15:43:58 dansmith mkay
15:44:12 efried dansmith: It's just because RequestGroup is used by both nova side and placement side.
15:44:26 efried on the placement side, we parse the incoming querystring into the exact same representation.
15:44:36 dansmith yeah, this seems like code sharing we should be removing so that we don't have any ties
15:44:41 efried It's like... having a serializable object without having a serializable object.
15:45:23 efried dansmith: Well, cdent is aware, and was involved in the review process (I think). I imagine there will come a time when there will be a placement_lib module that both of them will import.
15:45:58 dansmith I don't see why we'd use that to communicate between internal components of nova, unless it provides a lot of pre-calculation of things or something, which it does not do now,
15:46:22 dansmith but just be advised how hard it will be to land changes to that across both projects and update requirements and such before you can use a new thing if we go that route
15:46:34 cdent I think I expressed reservations at the time, but mostly shrugged in a "we'll figure it out" and "if it helps now, cool" kind of way.
15:46:50 dansmith this provides zero help in its current form, IMHO :)
15:47:13 efried dansmith: Well, only because we haven't closed the final switches on granular yet.
15:47:30 cdent It helps on the placement side to decode the query string, but on the nova side, dunno. I'm not paying huge amounts on the nova side as it is just too hard to keep track of _all_ things
15:47:49 cdent yeah, I think it is groundwork for stuff that was expected sooner than turned out
15:48:11 efried dansmith, cdent: On the nova side it lets us parse extra_specs; on the placement side it lets us parse the querystring. On both sides they parse into the same representation.
15:48:19 efried which is (or will be) useful).
15:48:22 efried ))
15:48:24 efried (((
15:48:25 dansmith cdent: it's just a class with a few variables righ tnow
15:48:41 dansmith anyway, I'm just pre-complaining, nothing that is going to block me right now
15:49:27 cdent dansmith: yeah, I know. I'm mostly speaking generally: I've de-prioritized my attention to the nova side of things for sake of being able to get anything done
15:55:10 openstackgerrit melissaml proposed openstack/nova master: fix a typo in service.py https://review.openstack.org/556575
15:56:00 openstackgerrit Merged openstack/nova master: Updated from global requirements https://review.openstack.org/556418
15:57:09 openstackgerrit Eric Berglund proposed openstack/nova master: PowerVM: Add proc_units_factor conf option https://review.openstack.org/554688
15:57:59 openstackgerrit Eric Berglund proposed openstack/nova master: PowerVM: Add proc_units_factor conf option https://review.openstack.org/554688
16:06:24 mriedem like killing mosquitoes https://review.openstack.org/#/c/556575/
16:06:40 mriedem https://review.openstack.org/#/c/545528/
16:07:17 mriedem https://review.openstack.org/#/c/555404/
16:07:18 mriedem geez
16:07:20 mriedem yay!
16:07:36 mriedem efried: "Note that some people are opposed to typo-in-comment-or-docstring patches. It's a religious thing, I think. So don't be surprised if this gets the kibosh." heh
16:08:28 mriedem i'm not opposed to fixing documentation if it makes the documentation more clear, i am opposed to stat padding py pushing several 1-line spell check fixes across openstack at random
16:08:34 mriedem *by pushing
16:10:29 efried mriedem: I thought we didn't put enough stock in stats to make that the sole reason for rejecting things like this. IMO patches like this could be fast-approved more quickly than they can be squashed. And let the author have the stats - what difference does it really make?
16:10:36 efried Anyway, that's my 2c
16:10:49 mriedem it's not the sole reason
16:10:54 stephenfin I'd be more lenient. If it doesn't cause merge conflicts, it's good and could be conceivably fast approved. If someone's basing their employee reviews on Stackalytics, they're the fools
16:10:57 mriedem it's also noise

Earlier   Later