| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2019-10-07 | |||
| 18:16:05 | efried | I guess we'll see. | |
| 18:27:32 | mriedem | melwitt: dansmith: am i overcomplicating this, or should the try/except here https://github.com/openstack/nova/blob/2c6542948f1f57c1a8a8faec1ba61800eaddff2a/nova/context.py#L442 be within the while loop? | |
| 18:27:48 | mriedem | e.g. the first result is a celltimeout but the 2nd yields results | |
| 18:28:41 | dansmith | mriedem: it won't be in the queue until it has timed out so I don't think it matters | |
| 18:29:01 | mriedem | ok so maybe just something i'm seeing in a unit test that is a problem | |
| 18:29:38 | dansmith | although I'm not sure where cell_uuid is coming from in that case | |
| 18:29:39 | mriedem | the order here https://github.com/openstack/nova/blob/1eec451b1ba254ec4c43c77f79b0d2809d6cb9a5/nova/tests/unit/test_context.py#L383 masks the regression being fixed here https://review.opendev.org/#/c/686996/ | |
| 18:30:01 | mriedem | because we get a result from cell0 so cell_uuid is in scope | |
| 18:30:30 | dansmith | well, that's a terrible reason of course :) | |
| 18:30:31 | mriedem | i've reversed the order of that side_effect and now https://github.com/openstack/nova/blob/1eec451b1ba254ec4c43c77f79b0d2809d6cb9a5/nova/tests/unit/test_context.py#L390 isn't in the results because of the try/except being outside the while loop | |
| 18:30:51 | dansmith | spawn as syncrhonous will make that very unrealistic | |
| 18:31:18 | mriedem | right so i'm just wondering if i chalk this up to fragile unit test and not try to have coverage for the regression case | |
| 18:31:36 | mriedem | or maybe i can just write a new test where the only thing we hit is a failure | |
| 18:31:46 | mriedem | that's probably easier to recreate the unboundlocalerror | |
| 18:41:16 | openstackgerrit | Matt Riedemann proposed openstack/nova master: [Gate fix] Avoid use cell_uuid before assignment https://review.opendev.org/686996 | |
| 18:42:46 | mriedem | ok ^ should be good | |
| 18:46:43 | dansmith | mriedem: my 202 API returns nothing, which seems to trip up novaclient | |
| 18:47:20 | dansmith | it's trying to turn the result into an aggregate I think, but it makes no sense for me to return an aggregate | |
| 18:47:47 | dansmith | I think I can do "give me the raw response" in novaclient, but I'm not sure that's the right thing to do | |
| 18:48:25 | dansmith | I'm going to trawl through some other 202 APIs to see if I can find one that doesn't return anything, but figured I'd poke in case you can just tell me what to do :) | |
| 18:49:29 | sean-k-mooney | dansmith: really i thought 202 ment accepted (generally for async calls) and it was normal for the body to be empty. | |
| 18:49:38 | dansmith | sean-k-mooney: me too | |
| 18:50:10 | dansmith | sean-k-mooney: although for a POST->202 I can see expecting to get back the thing you POST'd, maybe with validations, a uuid assigned, or something so I dunno | |
| 18:51:06 | sean-k-mooney | right i dont think haveing a body is wrong | |
| 18:51:12 | sean-k-mooney | i just would think it optional | |
| 18:51:17 | dansmith | yeah | |
| 18:51:23 | sean-k-mooney | i can totally see geting back a handel to the async task | |
| 18:51:24 | dansmith | might just be opinionated novaclient code, I dunno | |
| 18:53:06 | sean-k-mooney | assuming you dont find and relax the restiction in novaclient im sure you could just return {} | |
| 18:53:09 | sean-k-mooney | to make it happy | |
| 18:53:34 | dansmith | well, not if it's trying to turn the result into an aggregate, which is what I think it's doing | |
| 18:53:43 | dansmith | it chokes on the fact that it's None, but that's what it's trying to do | |
| 18:53:57 | mriedem | dansmith: my guess is the novaclient Aggregate class is just biased | |
| 18:54:06 | mriedem | "might just be opinionated novaclient code" yes this | |
| 18:54:17 | sean-k-mooney | ah right this is for your aggreate image endpoint | |
| 18:54:28 | dansmith | mriedem: ack, so, is it legit to just say raw_response=True so I can get it to chillax? | |
| 18:54:33 | mriedem | i think so | |
| 18:54:33 | efried | mriedem: unbound local, RC2 yah? | |
| 18:54:39 | mriedem | efried: no, it's not in train | |
| 18:54:42 | efried | owait, we didn't put it in train. | |
| 18:54:45 | efried | sorry | |
| 18:55:58 | mriedem | dansmith: you'll also have to pass an object_class | |
| 18:56:04 | mriedem | i think | |
| 18:56:13 | mriedem | otherwise it's going to try and shove it into an Aggregate object | |
| 18:56:15 | dansmith | not for return_raw=True, by my reading | |
| 18:56:27 | mriedem | obj_class = self.resource_class | |
| 18:56:27 | mriedem | if obj_class is None: | |
| 18:56:30 | mriedem | maybe not | |
| 18:56:38 | mriedem | but if there is no body... | |
| 18:56:40 | dansmith | return_raw has returned by then | |
| 18:56:47 | dansmith | ah yeah, guh | |
| 18:56:48 | mriedem | oh right | |
| 18:56:49 | mriedem | yeah | |
| 18:56:53 | mriedem | er no | |
| 18:56:54 | mriedem | :) | |
| 18:57:01 | mriedem | i literally don't trust my eyes right now | |
| 18:57:21 | dansmith | can I just call self.api.client.post myself? | |
| 18:58:24 | sean-k-mooney | i think we do that in a few places | |
| 18:58:45 | sean-k-mooney | althout i have only done that in func tests | |
| 18:58:49 | mriedem | it looks like if you don't, _create expects a body in the response | |
| 18:59:01 | mriedem | sean-k-mooney: you know we're talking about python-novaclient here? | |
| 18:59:25 | mriedem | anyway i have to jump on a call | |
| 18:59:41 | dansmith | mriedem: ack, I'll just do that and wait for reviewers to tell me no | |
| 19:00:54 | sean-k-mooney | mriedem: right fair point i was thinking about usage in nova. | |
| 19:03:31 | dansmith | the good news, at least, | |
| 19:03:51 | dansmith | is that this is all just duck-alignment and that the whole thing works and pre-caches the image as expected | |
| 19:04:16 | sean-k-mooney | :) | |
| 19:04:51 | sean-k-mooney | have you implmented the trotteling code or just the basic poc | |
| 19:05:38 | dansmith | I've done the parallelism=N stuff yeah | |
| 19:06:05 | sean-k-mooney | cool | |
| 19:06:08 | dansmith | I'm going to push up in a sec so I can review it myself and look for tests I'm missing (of which there are many) | |
| 19:06:58 | sean-k-mooney | its funny but i often find the change in media between my ide and the gerrit review helps with things like that | |
| 19:07:09 | dansmith | yes, exactly | |
| 19:07:18 | openstackgerrit | Dan Smith proposed openstack/nova master: Add cache_image() support to the compute rpc, api, and manager https://review.opendev.org/687138 | |
| 19:07:18 | openstackgerrit | Dan Smith proposed openstack/nova master: Add cache_image() driver method and libvirt implementation https://review.opendev.org/687137 | |
| 19:07:19 | openstackgerrit | Dan Smith proposed openstack/nova master: WIP: Add image caching API for aggregates https://review.opendev.org/687140 | |
| 19:07:19 | openstackgerrit | Dan Smith proposed openstack/nova master: Add cache_images() to conductor https://review.opendev.org/687139 | |
| 19:07:29 | dansmith | gerrit is where I'm used to looking for those things | |
| 19:08:12 | openstackgerrit | Dan Smith proposed openstack/python-novaclient master: WIP: Add aggregate-cache-images command and client routines https://review.opendev.org/687141 | |
| 19:09:10 | sean-k-mooney | by the way you are not storing the cached state of the image anywhere correct? that is just known on the compute nodes | |
| 19:09:22 | dansmith | I'm really tempted to add a "check only" flag so you can single-shot the api and just say "check and see which of these aren't done to give at least the tiniest bit of reporting, but I think I need to resist the urge | |
| 19:09:29 | dansmith | correct | |
| 19:09:32 | dansmith | per the spec :) | |
| 19:10:14 | sean-k-mooney | yep im rebasing my schduler prefilter code to master and just wanted to confirm in my head that it should not conflict | |
| 19:10:19 | dansmith | many of the previous attempts stalled out at the persistence layer so I definitely want to punt on that | |
| 19:11:17 | sean-k-mooney | ya the tangent my brain was one was could you use a weighr to prefer the hosts wher eit was cached by reporting the cached images in the hostSate object | |
| 19:11:55 | sean-k-mooney | anyway not important now | |
| 19:12:42 | dansmith | sean-k-mooney: a weigher for exactly that is one of the ill-fated "previous attempt" items at the bottom of the spec | |
| 19:13:19 | sean-k-mooney | ah i see well there is nothing preventing it being added at a future point in your design | |
| 19:13:20 | dansmith | a sunken treasure for you | |
| 19:15:26 | dansmith | I want to roll this out and hear the requests and justifications for more complicated stuff | |
| 19:15:31 | sean-k-mooney | ah https://blueprints.launchpad.net/nova/+spec/node-cached-image-weigher | |
| 19:16:07 | sean-k-mooney | ya makes sense | |
| 19:30:26 | openstackgerrit | sean mooney proposed openstack/nova master: Libvirt: report storage bus traits https://review.opendev.org/666914 | |
| 19:30:27 | openstackgerrit | sean mooney proposed openstack/nova master: Add transform_image_metadata request filter https://review.opendev.org/665775 | |
| 19:30:27 | openstackgerrit | sean mooney proposed openstack/nova master: libvirt: use domain capabilities to get supported device models https://review.opendev.org/666915 | |
| 19:31:24 | sean-k-mooney | efried: do you mind removing your -2 from https://review.opendev.org/#/c/666914/ now that master is open for ussuri | |
| 19:32:08 | efried | sean-k-mooney: will do, but I'm going to put the bp "Direction" back to pending | |
| 19:32:30 | efried | done | |
| 19:32:48 | sean-k-mooney | efried: ok. | |