| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2019-08-30 | |||
| 12:22:44 | lyarwood | aaaaaaah that's already mocking it in setup | |
| 12:22:47 | lyarwood | my bad | |
| 12:23:19 | sean-k-mooney | this | |
| 12:23:21 | sean-k-mooney | self.useFixture(fixtures.MonkeyPatch( | |
| 12:23:23 | sean-k-mooney | 'nova.virt.libvirt.host.libvirt', | |
| 12:23:25 | sean-k-mooney | fakelibvirt)) | |
| 12:24:02 | sean-k-mooney | when you do | |
| 12:24:03 | sean-k-mooney | https://github.com/openstack/nova/blob/master/nova/tests/functional/regressions/test_bug_1595962.py#L41 | |
| 12:24:17 | lyarwood | No, lower | |
| 12:24:23 | lyarwood | # Mock the 'get_connection' function, as we're going to need to provide | |
| 12:24:24 | lyarwood | # custom capabilities for each test | |
| 12:24:26 | lyarwood | _p = mock.patch('nova.virt.libvirt.host.Host.get_connection') | |
| 12:24:28 | lyarwood | self.mock_conn = _p.start() | |
| 12:24:30 | lyarwood | self.addCleanup(_p.stop) | |
| 12:25:29 | sean-k-mooney | i dont see that in the file you linked but sounds like you have figured it out | |
| 12:27:04 | lyarwood | sean-k-mooney: sorry that's in the base class I'm using in the second link | |
| 12:27:12 | lyarwood | sean-k-mooney: and yeah it's working now | |
| 12:41:22 | openstackgerrit | François Palin proposed openstack/nova master: Add retry when attachment_delete or terminate_connection fails https://review.opendev.org/669674 | |
| 12:45:21 | openstackgerrit | sean mooney proposed openstack/nova master: fix lxml compatibility issues https://review.opendev.org/678964 | |
| 12:45:58 | sean-k-mooney | prometheanfire: efried ^ now using self.assertXmlEqual | |
| 12:46:17 | aspiers | sean-k-mooney: \o/ | |
| 12:46:31 | aspiers | glad I invested some time into that function | |
| 12:46:45 | sean-k-mooney | i was doing it the old way | |
| 12:46:54 | sean-k-mooney | then erric remided me it existited | |
| 12:47:24 | sean-k-mooney | i dont know why testtoold decied that matches shoudl return nothing for sucess | |
| 12:47:44 | sean-k-mooney | but i hate that api so im glad we have a wrapper | |
| 12:48:33 | sean-k-mooney | prometheanfire: actully we need to invert the depends on relationship | |
| 12:48:51 | sean-k-mooney | your upperconstaits bump needs to depend on the nova change | |
| 12:49:22 | openstackgerrit | sean mooney proposed openstack/nova master: fix lxml compatibility issues https://review.opendev.org/678964 | |
| 12:49:27 | efried | sean-k-mooney: +2, thank you | |
| 12:49:51 | efried | race condition | |
| 12:49:51 | sean-k-mooney | efried: hehe care to +2 again | |
| 12:49:54 | efried | done | |
| 12:49:59 | sean-k-mooney | thanks | |
| 12:50:09 | sean-k-mooney | prometheanfire: do you mind if i update your patch to depend on mine | |
| 12:50:13 | efried | since you were updating the commit message, you could have s/XMLMatcher/assertXmlEqaual/ | |
| 12:50:23 | efried | but technically still true, so meh | |
| 12:50:31 | sean-k-mooney | oh i can if you want | |
| 12:50:34 | efried | nah | |
| 12:50:37 | sean-k-mooney | ok | |
| 12:51:58 | luyao | dansmith: Are you around? | |
| 12:56:21 | mriedem | luyao: he's out today | |
| 12:58:12 | luyao | mriedem: okey, thanks for telling me this | |
| 12:58:51 | sean-k-mooney | mriedem: i finally got around to updating https://review.opendev.org/#/c/669738/. the functional tests are now in there own module as you asked. sorry for the delay. | |
| 12:59:03 | gibi | mriedem: hi! thanks for the reviews on the nova-grenade-multinode patches. It seems that GRENADE_PHASE var is not defined even in a grenade job. but GRENADE_NEW_BRANCH is, so I'm pushing a new patch set soon | |
| 12:59:10 | sean-k-mooney | oh i added a shelve/unshelve test too | |
| 12:59:49 | mriedem | gibi: hmm, when i looked at the job results yesterday i thought i looked for that, but can't remember the details now | |
| 13:00:23 | gibi | mriedem: here is a run with env dumped https://b8c08d7c14d41636ca2c-db936121e3703832159c63a0fb95ab4b.ssl.cf5.rackcdn.com/679210/5/check/nova-grenade-multinode/2489dbd/job-output.txt | |
| 13:00:35 | openstackgerrit | Balazs Gibizer proposed openstack/nova master: Add cold migrate and resize to nova-grenade-multinode https://review.opendev.org/679210 | |
| 13:00:54 | mriedem | gibi: GRENADE_(NEW|OLD)_BRANCH should work yeah, we've used that before https://github.com/openstack/nova/blob/stable/pike/nova/tests/live_migration/hooks/run_tests.sh#L52 | |
| 13:00:58 | luyao | mriedem: Welcome to review this patch series if you would like, I'll appreciate that. Current design is NOT virt driver specific. https://review.opendev.org/#/q/topic:bp/virtual-persistent-memory+(status:open) | |
| 13:02:08 | mriedem | gibi: ok you're going to drop that env dump yeah? | |
| 13:02:10 | mriedem | that's just WIP? | |
| 13:02:25 | gibi | mriedem: yes, I just noticed it | |
| 13:02:31 | gibi | mriedem: new PS is coming | |
| 13:02:56 | mriedem | luyao: i'll be honest with you, that series is likely not something i'm going to be focusing on in the next 2 weeks to feature freeze, i'm already committed to reviewing some other series that have been around longer | |
| 13:03:33 | openstackgerrit | Balazs Gibizer proposed openstack/nova master: Add cold migrate and resize to nova-grenade-multinode https://review.opendev.org/679210 | |
| 13:03:38 | mriedem | sean-k-mooney: btw, totally unrelated, i realize our docs for AggregateInstanceExtraSpecsFilter kind of suck https://docs.openstack.org/nova/latest/admin/configuration/schedulers.html?highlight=aggregate_instance_extra_specs#aggregateinstanceextraspecsfilter | |
| 13:04:00 | mriedem | meaning, what the hell is aggregate_instance_extra_specs supposed to mean to a user trying to configure flavors or aggregates for that filter? | |
| 13:04:10 | mriedem | looking at the code it doesn't even really make sense to me | |
| 13:04:35 | sean-k-mooney | i know i hate that filter and tried to replaces it | |
| 13:04:59 | mriedem | should at least have an example of how to use it | |
| 13:05:08 | sean-k-mooney | yes | |
| 13:05:08 | mriedem | it came up while triaging this bug https://bugs.launchpad.net/nova/+bug/1841932 | |
| 13:05:09 | openstack | Launchpad bug 1841932 in OpenStack Compute (nova) "hide_hypervisor_id extra_specs in nova flavor cannot pass AggregateInstanceExtraSpecsFilter" [Undecided,Incomplete] | |
| 13:05:20 | sean-k-mooney | i can proably add one if you want | |
| 13:05:49 | luyao | mriedem: Yes, I fully understand you, but any comments are welcome, even small and short. :) | |
| 13:07:47 | sean-k-mooney | mriedem: it bassicaly asserts that that every extraspec that is in the fiter (optionally that starts with aggregate_instance_extra_specs:) is set in at least 1 aggreate the current host is a member of | |
| 13:07:52 | openstackgerrit | Adam Spiers proposed openstack/nova master: Add extra spec parameter and image property for memory encryption https://review.opendev.org/664420 | |
| 13:08:06 | aspiers | alex_xu, stephenfin: Your feedback addressed ^^^ | |
| 13:09:50 | sean-k-mooney | mriedem: looking at the bug report its working as intended | |
| 13:10:05 | sean-k-mooney | well actuly | |
| 13:11:07 | sean-k-mooney | it might not handel un namepaced extra specs properly ill check | |
| 13:12:10 | sean-k-mooney | mriedem: no its working as indended | |
| 13:12:38 | sean-k-mooney | its not working as they want in https://bugs.launchpad.net/nova/+bug/1841932 | |
| 13:12:38 | openstack | Launchpad bug 1841932 in OpenStack Compute (nova) "hide_hypervisor_id extra_specs in nova flavor cannot pass AggregateInstanceExtraSpecsFilter" [Undecided,Incomplete] | |
| 13:12:52 | sean-k-mooney | but that is worrking the way that filter has always worked | |
| 13:12:57 | mriedem | sean-k-mooney: i've reported a docs bug if you you're keen https://bugs.launchpad.net/nova/+bug/1842061 | |
| 13:12:58 | openstack | Launchpad bug 1842061 in OpenStack Compute (nova) "Compute schedulers in nova - AggregateInstanceExtraSpecsFilter docs are not clear" [Undecided,New] | |
| 13:15:01 | sean-k-mooney | ill see if i can add it to my list. but we create https://github.com/openstack/nfv-filters/blob/master/nfv_filters/nova/scheduler/filters/aggregate_instance_type_filter.py to replace it for several reason. also with docs https://github.com/openstack/nfv-filters/blob/master/doc/source/scheduler_filters/aggregate-instance-type-filter.rst | |
| 13:15:16 | aspiers | stephenfin: I also split off those fixups | |
| 13:29:08 | efried | aspiers: why is textwrap.dedent necessary https://review.opendev.org/#/c/679339/3/nova/tests/unit/virt/libvirt/fake_libvirt_data.py ? | |
| 13:29:16 | openstackgerrit | Merged openstack/nova-specs master: Create specs directory for Ussuri https://review.opendev.org/679364 | |
| 13:29:20 | sean-k-mooney | efried: its not | |
| 13:29:21 | efried | This is xml, not python - it doesn't care about whitespace internally, does it? | |
| 13:29:32 | sean-k-mooney | actully it does | |
| 13:29:32 | aspiers | efried: it's necessary to keep stephenfin happy, and I kinda like it too | |
| 13:29:51 | sean-k-mooney | but we are using xml parsing libs and we dont care | |
| 13:30:03 | aspiers | I could live without, but he suggested it and I like keeping things as readable as possible | |
| 13:30:10 | aspiers | sean-k-mooney: it's about readability not parsing | |
| 13:30:14 | efried | I'm not asking why you're indenting the lines with the xml on them. Visually prettier. I'm asking why textwrap.dedent is necessary. | |
| 13:30:17 | stephenfin | hmm, yeah, we could forget about the textwrap piece, actually. It was the indentation of the XML itself that I cared about | |
| 13:30:23 | aspiers | Oh right | |
| 13:30:28 | sean-k-mooney | aspiers: yes but we dont read the xml we read the code | |
| 13:30:30 | stephenfin | efried: I suggested that out of habit | |
| 13:30:37 | aspiers | sean-k-mooney: strongly disagree | |
| 13:30:39 | stephenfin | You're right, it's not needed in this case | |
| 13:30:43 | sean-k-mooney | and that makes the code mroe readable | |
| 13:30:49 | aspiers | I can drop dedent if that's the consensus | |
| 13:30:49 | sean-k-mooney | e.g. leaving it out | |