| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2018-03-30 | |||
| 17:43:02 | claudiub|2 | fried_bunny: added: https://review.openstack.org/#/c/557974/ | |
| 17:43:23 | claudiub|2 | can you run the UT with this patch applied? | |
| 17:43:49 | fried_bunny | uhm, I think by cloning the project into my tox env... | |
| 17:43:51 | fried_bunny | stand by | |
| 17:44:21 | claudiub|2 | yeah. it seems to work for me. Want to double check. | |
| 17:44:58 | fried_bunny | claudiub|2: Are you running tox on nova-powervm? | |
| 17:45:29 | claudiub|2 | I ran an individual test: python -m unittest nova_powervm.tests.virt.powervm.disk.test_ssp.TestSSPDiskAdapter.test_connect_disk | |
| 17:45:37 | fried_bunny | ight | |
| 17:45:38 | claudiub|2 | this was one of the failures | |
| 17:45:46 | fried_bunny | I'll run the whole suite, sec. | |
| 17:45:53 | claudiub|2 | yeap, please do | |
| 17:46:23 | fried_bunny | claudiub|2: Is this a LP bug thing, or no? | |
| 17:46:28 | openstackgerrit | Merged openstack/nova stable/pike: Re-use existing ComputeNode on ironic rebalance https://review.openstack.org/527423 | |
| 17:46:34 | openstackgerrit | Merged openstack/nova stable/pike: Do not set allocation.id in AllocationList.create_all() https://review.openstack.org/530794 | |
| 17:46:40 | openstackgerrit | Merged openstack/nova stable/pike: Don't launch guestfs in a thread pool if guestfs.debug is enabled https://review.openstack.org/539541 | |
| 17:46:47 | openstackgerrit | Merged openstack/nova stable/pike: unquiesce instance on volume snapshot failure https://review.openstack.org/545966 | |
| 17:46:53 | fried_bunny | claudiub|2, edmondsw: Confirmed, that delta fixes us completely. | |
| 17:46:54 | claudiub|2 | I'll amend the commit message and add a Related-Bug: #the_mock_autospec_LP_bug_id | |
| 17:47:11 | fried_bunny | roger that. | |
| 17:47:20 | claudiub|2 | ok, then i'll amend the commit message | |
| 17:47:58 | claudiub|2 | gonna push for it to merge, and ask for a new oslotest release next week. they don't release at the end of the week. | |
| 17:48:49 | claudiub|2 | sorry for the trouble. :) | |
| 17:49:06 | fried_bunny | claudiub|2: No worries; that was pretty unanticipatable. Thank you very much for the help. | |
| 17:55:55 | claudiub|2 | updated commit message | |
| 17:56:03 | edmondsw | claudiub|2 good catch and thanks | |
| 18:13:53 | fried_bunny | claudiub|2: Finding those mocked attrs wasn't as hard as I thought. | |
| 18:13:53 | fried_bunny | Eric Fried proposed openstack/nova-powervm master: Specify UUIDs in VIOS mocks for FeedTasks https://review.openstack.org/557975 | |
| 18:14:19 | fried_bunny | If you want to pull your patch, I think I'd be okay with that. (edmondsw, thoughts?) | |
| 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 | stephenfin | melwitt: RE: bug 1750672, I already fixed that with https://review.openstack.org/#/c/556894/2 | |
| 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: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 | with mock.patch.object(self.compute, 'virtapi') as fake_virtapi: | |
| 20:39:13 | mriedem | fake_virtapi.return_value.wait_for_instance_event.side_effect = exc | |
| 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 | with mock.patch.object(self.compute.virtapi, | |
| 20:42:40 | mriedem | 'wait_for_instance_event') as wait_for_event: | |
| 20:42:40 | mriedem | wait_for_event.return_value.__enter__.return_value.side_effect = exc | |
| 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 | with mock.patch.object(self.compute.virtapi, 'wait_for_instance_event') as wait_for_event: | |
| 20:51:26 | fried_bunny | wait_for_event.return_value.__enter__.side_effect = exc | |
| 20:51:41 | fried_bunny | you had a different mistake in each attempt. | |
| 20:51:52 | mriedem | i just tried that, | |