| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2018-03-30 | |||
| 18:15:39 | edmondsw | might still be a good idea to keep claudiub's patch so that this weirdness doesn't crop up in other places | |
| 18:16:05 | fried_bunny | I'm fine with that too. Weighed against the desire to keep those classes private, though - that's up to claudiub|2 I guess. | |
| 18:16:09 | mriedem | why are nova-powervm repo changes showing up in the nova channel? | |
| 18:16:21 | fried_bunny | mriedem: Because triggered by a nova change | |
| 18:16:34 | mriedem | ? | |
| 18:16:39 | mriedem | https://review.openstack.org/#/c/557975/ isn't nova | |
| 18:16:41 | fried_bunny | mriedem: Oh, that wasn't a bot, that was a copy/paste by me. | |
| 18:16:50 | mriedem | ah | |
| 18:16:51 | fried_bunny | thought you were going crazy | |
| 18:17:40 | fried_bunny | mriedem: We were discussing here because the trigger was https://review.openstack.org/#/c/447505/ | |
| 18:17:55 | mriedem | yar | |
| 18:18:14 | mriedem | i noticed an alarming amount of claudiub|2 in the channel | |
| 18:18:29 | claudiub|2 | fried_bunny: edmondsw: this should also work, as a temporary hack: https://review.openstack.org/557976 | |
| 18:18:47 | fried_bunny | claudiub|2: Cool, that's useful to know. Thanks. | |
| 18:19:29 | fried_bunny | claudiub|2: If https://review.openstack.org/557975 doesn't fix us, we'll use that. | |
| 18:19:45 | claudiub|2 | gotcha. :) | |
| 18:20:10 | claudiub|2 | mriedem: yeah, usually when there's a fire somewhere. :D | |
| 18:20:43 | claudiub|2 | because you know, a lot of fire leads to getting fired. ha. | |
| 18:21:02 | mriedem | it is that time of the quarter | |
| 18:35:45 | openstack | bug 1750672 in OpenStack Compute (nova) "failure to generate Nova's doc in Python 3.6" [Medium,Confirmed] https://launchpad.net/bugs/1750672 | |
| 18:35:45 | stephenfin | melwitt: RE: bug 1750672, I already fixed that with https://review.openstack.org/#/c/556894/2 | |
| 18:37:05 | openstackgerrit | Merged openstack/nova stable/pike: Return 400 when compute host is not found https://review.openstack.org/550707 | |
| 18:57:28 | fried_bunny | melwitt: On vacation, yah? | |
| 18:58:12 | mriedem | yes, mandatory pto | |
| 19:03:21 | fried_bunny | leakypipes: I answered here https://review.openstack.org/#/c/533821/ -- in your estimation was there something more she was expecting to see in the tests? | |
| 19:11:43 | openstackgerrit | Merged openstack/nova stable/pike: Always deallocate networking before reschedule if using Neutron https://review.openstack.org/555907 | |
| 19:20:10 | fried_bunny | leakypipes: Likewise https://review.openstack.org/#/c/520246/ | |
| 19:27:28 | openstackgerrit | Takashi NATSUME proposed openstack/nova master: Remove unnecessary code encoding specification https://review.openstack.org/557903 | |
| 19:53:36 | leakypipes | fried_bunny: yeah, I'm just not sure :( | |
| 20:05:48 | fried_bunny | mriedem: If you were waiting for my nod on https://review.openstack.org/#/c/553122/ it's done. | |
| 20:06:12 | mriedem | ack | |
| 20:13:12 | openstackgerrit | Arvind Nadendla proposed openstack/nova master: Update ImageMetaProp object to expose traits https://review.openstack.org/557795 | |
| 20:16:57 | mriedem | gah, trying to mock context managers, my old nemesis | |
| 20:20:09 | openstackgerrit | Arvind Nadendla proposed openstack/nova master: Update ImageMetaProp object to expose traits https://review.openstack.org/557795 | |
| 20:29:58 | fried_bunny | mriedem: Ditto https://review.openstack.org/#/c/533396/ | |
| 20:30:07 | fried_bunny | mriedem: I can help with that if you like. Or were you just grumbling? | |
| 20:30:52 | mriedem | i think i've got a way around it | |
| 20:39:12 | mriedem | ugh | |
| 20:39:13 | mriedem | fake_virtapi.return_value.wait_for_instance_event.side_effect = exc | |
| 20:39:13 | mriedem | with mock.patch.object(self.compute, 'virtapi') as fake_virtapi: | |
| 20:39:19 | mriedem | i must be blind | |
| 20:39:32 | mriedem | that is not hitting | |
| 20:42:39 | mriedem | or | |
| 20:42:40 | mriedem | wait_for_event.return_value.__enter__.return_value.side_effect = exc | |
| 20:42:40 | mriedem | 'wait_for_instance_event') as wait_for_event: | |
| 20:42:40 | mriedem | with mock.patch.object(self.compute.virtapi, | |
| 20:43:29 | fried_bunny | mriedem: Is self.compute.virtapi a method or a property? | |
| 20:43:42 | mriedem | it's an attribute | |
| 20:43:48 | fried_bunny | mriedem: Then take out .return_value | |
| 20:43:49 | mriedem | with a wait_for_instance_event method | |
| 20:44:14 | fried_bunny | (in the first paste) | |
| 20:46:39 | mriedem | i don't think the first one will work, | |
| 20:46:48 | mriedem | wait_for_instance_event is a context manager, | |
| 20:46:52 | mriedem | so need to mock the __enter__ | |
| 20:47:41 | fried_bunny | okay, but fake_virtapi.return_value will never hit, because it's not self.compute.virtapi().wait_for_instance_event - just self.compute.virtapi.wait_for_instance_event, right? | |
| 20:47:51 | fried_bunny | so that's a start... | |
| 20:48:16 | mriedem | right | |
| 20:48:46 | fried_bunny | mriedem: Is it FakeVirtAPI you're actually using here? | |
| 20:48:52 | mriedem | no | |
| 20:48:59 | fried_bunny | ComputeVirtAPI? | |
| 20:49:00 | mriedem | also, trying to model this after https://github.com/openstack/nova/blob/master/nova/tests/unit/compute/test_compute_api.py#L1749 | |
| 20:49:07 | mriedem | yeah it's ComputeVirtAPI in this case | |
| 20:49:58 | mriedem | i don't think using FakeVirtAPI will help, since that also defines wait_for_instance_event as a context manager | |
| 20:50:00 | mriedem | it's just a noop | |
| 20:50:25 | openstackgerrit | Arvind Nadendla proposed openstack/nova master: Update ImageMetaProp object to expose traits https://review.openstack.org/557795 | |
| 20:50:27 | fried_bunny | mriedem: No, was just going to suggest the possibility of using a mock.patch('....ComputeVirtAPI') instead of a mock.patch.object. | |
| 20:50:46 | fried_bunny | mriedem: But okay, I think I see the problem. | |
| 20:50:51 | fried_bunny | Try this: | |
| 20:51:26 | fried_bunny | wait_for_event.return_value.__enter__.side_effect = exc | |
| 20:51:26 | fried_bunny | with mock.patch.object(self.compute.virtapi, 'wait_for_instance_event') as wait_for_event: | |
| 20:51:41 | fried_bunny | you had a different mistake in each attempt. | |
| 20:51:52 | mriedem | i just tried that, | |
| 20:51:59 | mriedem | believe me, i've tried about every iteration of this | |
| 20:52:07 | mriedem | anyway, trying something else here | |
| 20:54:12 | fried_bunny | mriedem: I'm not sure that mocking wait_for_instance_event as if it were a context manager is going to do what you expect. | |
| 20:54:25 | fried_bunny | (well, clearly not) | |
| 20:54:40 | fried_bunny | because it's using the decorator instead of being a real context manager. | |
| 20:55:12 | fried_bunny | w4ie.side_effect = exc | |
| 20:55:12 | fried_bunny | with mock.patch('nova.compute.manager.ComputeVirtAPI#wait_for_instance_event') as w4ie: | |
| 20:55:12 | fried_bunny | You could try | |
| 20:55:22 | fried_bunny | s/#/./ | |
| 20:55:41 | mriedem | as far as i can tell, what i'm doing is basically the same as https://github.com/openstack/nova/blob/master/nova/tests/unit/compute/test_compute_api.py#L1749 | |
| 20:55:46 | mriedem | just a different context manager | |
| 20:56:15 | fried_bunny | ahh | |
| 20:56:28 | mriedem | but wait a sec, i might have something else screwed up here | |
| 20:56:48 | fried_bunny | I don't actually know whether side_effect does the right thing for yield. Though I guess it should... | |
| 21:03:18 | mriedem | got it | |
| 21:03:28 | mriedem | it wasn't even the mock, it was the thing i was using as the exc | |
| 21:03:35 | mriedem | i knew it would be dumb | |
| 21:03:55 | fried_bunny | mriedem: Good, because I just tried it out in mini form and it's all working like it's sposedta | |
| 21:18:23 | imacdonn | mriedem: around ? | |
| 21:24:32 | mriedem | depends | |
| 21:24:50 | imacdonn | mriedem: heh ... looking at https://review.openstack.org/#/c/554759/ | |
| 21:25:07 | imacdonn | mriedem: I think that's the wrong fix ... I think the right fix is to remove the check entirely | |
| 21:26:04 | mriedem | ? | |
| 21:26:08 | mriedem | placement is required to start nova-compute | |
| 21:26:11 | mriedem | since ocata | |
| 21:26:11 | openstack | Launchpad bug 1751692 in OpenStack Compute (nova) "os_region_name an unnecessary required option for placement " [Low,Triaged] - Assigned to Digambar (digambarpatil15) | |
| 21:26:11 | imacdonn | mriedem: see Sylvain's comment on https://bugs.launchpad.net/nova/+bug/1751692 | |
| 21:26:51 | imacdonn | Isn't the recommendation to only specify region_name if you need to override what keystone offers ? | |