Earlier  
Posted Nick Remark
#openstack-nova - 2018-04-17
14:53:51 mlavalle and didn't think about us ;-)
14:53:57 efried kashyap: Interestingly, both versions of the test fail the same way.
14:54:10 kashyap efried: You mean, my so-called "updated" and the old one?
14:54:32 efried kashyap: Yes. And there's no trace to actual running code; I suspect it's blowing up at mock time.
14:54:43 kashyap efried: Exactly
14:55:03 kashyap I have no idea, my Mock knowledge is ... mockable
14:55:08 efried kashyap: I mean it's the decorators.
14:55:15 stephenfin mriedem: Is the API in nova/network/base_api.py a contract we need to maintain for external users?
14:55:31 stephenfin i.e. N cycles before we can remove a function
14:55:33 efried kashyap: which is bizarro because the original don't have no decorators.
14:55:38 gibi mlavalle: yes, It was really nice and I was able to forget about work :)
14:55:40 kashyap efried: Fantastici
14:55:44 kashyap efried: But you know --
14:55:54 gibi mlavalle: now I'm trying to catch up
14:56:07 mlavalle :-)
14:56:25 efried kashyap: f me this test suite is huge.
14:56:45 kashyap efried: Tell me all about it
14:57:19 kashyap efried: For context, see this -- "test_live_migration_update_volume_xml", this is almost same as the other one: test_live_migration_with_valid_target_connect_addr
15:00:36 efried uh oh, my IRC client is freaking out. Back in a tick...
15:01:39 openstackgerrit Merged openstack/nova master: Transform aggregate.update_metadata notification https://review.openstack.org/460625
15:01:47 kashyap mriedem: Are such unit test only patches frowned-upon? https://review.openstack.org/#/c/561969/
15:02:26 kashyap mriedem: I've only got two of such, though. And two more to go in that vien -- fixing the fallout of migrateToURI{2,3} APIs
15:02:51 openstackgerrit Chris Dent proposed openstack/nova master: Optional separate database for placement API https://review.openstack.org/362766
15:02:52 openstackgerrit Chris Dent proposed openstack/nova master: Isolate placement database config https://review.openstack.org/541435
15:02:53 openstackgerrit Chris Dent proposed openstack/nova master: WIP: Ensure that os-traits sync is attempted only at start of process https://review.openstack.org/553857
15:03:35 openstackgerrit sahid proposed openstack/nova master: libvirt: fix wrong driver name for vhostuser interface https://review.openstack.org/561562
15:03:36 openstackgerrit sahid proposed openstack/nova master: libvirt: add support for virtio-net rx/tx queue sizes https://review.openstack.org/484997
15:04:54 mriedem umm
15:05:07 mriedem kashyap: the commit message doesn't explain to me why we need those changes
15:05:16 mriedem if the tests aren't failing as a result of the version cap changes
15:06:22 mriedem -1 with words to mull
15:06:35 kashyap mriedem: They are of course failing due to the version cap changes.
15:06:42 kashyap Not fixing them out of giggles
15:06:53 kashyap mriedem: But yes, fair point on the commit message -- I'll write that
15:07:48 mriedem so these changes are coming before some future patch that changes/removes some version-dependent piece of code?
15:07:55 mriedem rather than just include them in that same change?
15:07:58 kashyap With the version bump, we get the migrateToURI3() API for gratis, and the older one is riddled with problems
15:08:20 mriedem if the tests start to fail b/c you remove x, then include them in the patch that removes x
15:08:22 kashyap mriedem: Yeah. I thought including them all in one big blob is ugly
15:08:31 kashyap But if that's preferred, I can squash them into that change
15:08:48 kashyap Because probably it *belongs* with the bump change, as that is what has caused the problem.
15:09:15 mriedem typically the only test only changes i see are (1) adding more test coverage to existing code or (2) a negative test to recreate a bug which is then followed up by a patch that shows the bug is fixed
15:10:21 kashyap mriedem: Yeah, I noticed it too, from Git history sleuthing. And it felt "dirty" to just publish a test-only change
15:10:31 kashyap I'll squash them in. Thanks for the comment
15:15:35 openstackgerrit Konstantinos Samaras-Tsakiris proposed openstack/nova master: Add `hide_hypervisor_id` flavor extra_spec https://review.openstack.org/555861
15:22:04 openstackgerrit sahid proposed openstack/nova master: compute: introduce cpu_share_set option https://review.openstack.org/561912
15:22:05 openstackgerrit sahid proposed openstack/nova master: libvirt: pass emulator threads policy instead whether isolated https://review.openstack.org/561913
15:22:06 openstackgerrit sahid proposed openstack/nova master: libvirt: place emulator threads on CONF.cpu_share_set https://review.openstack.org/510897
15:38:31 sean-k-mooney mriedem: on the topic of test only changes, is nova planning to remove mox this cycle or are we just rewriting the test that use mox as alter the code
15:40:00 mriedem https://blueprints.launchpad.net/nova/+spec/mox-removal
15:40:50 sean-k-mooney mriedem: yet another thing the depens on removal of nova networks to finish :)
15:55:09 arvindn05 bauzas: jaypipes: https://review.openstack.org/#/c/560718/ - can we discuss this spec?
15:55:15 openstackgerrit Matt Riedemann proposed openstack/nova master: Cleanup ugly stub in TestLocalDeleteAllocations https://review.openstack.org/561997
15:56:56 openstackgerrit Matt Riedemann proposed openstack/nova master: Cleanup ugly stub in TestLocalDeleteAllocations https://review.openstack.org/561997
15:58:11 jaypipes arvindn05: sure
15:59:22 arvindn05 jaypipes: ty...its hard to find bauzas in IRC because of timezone...if you know of suggestions for when i can find him in IRC that would help :)
15:59:38 bauzas arvindn05: jaypipes: I'm here but it's 6pm here
15:59:46 bauzas so I'll litterally disappear in like the next mins
16:00:00 arvindn05 ahh...its 9 AM here...
16:00:17 arvindn05 will try to catch you earlier next time then
16:00:26 bauzas arvindn05: you're US based ?
16:00:31 bauzas West coast I guess
16:00:39 arvindn05 yup...CA
16:01:11 openstackgerrit Merged openstack/nova stable/ocata: Add regression test for BFV+IsolatedHostsFilter failure https://review.openstack.org/543634
16:02:15 arvindn05 bauzas: please do respond to the comment offline if you get a chance....my second patch for the BP depends on this change so it is starting to block progress for me
16:02:42 openstackgerrit Ed Leafe proposed openstack/nova master: Add columns for generation to consumers https://review.openstack.org/557958
16:02:43 openstackgerrit Ed Leafe proposed openstack/nova master: Add the Consumer object https://review.openstack.org/561406
16:02:44 openstackgerrit Ed Leafe proposed openstack/nova master: Add Consumer objects to Allocations https://review.openstack.org/561407
16:02:45 openstackgerrit Ed Leafe proposed openstack/nova master: Add generation checking logic to Consumer https://review.openstack.org/561408
16:02:46 openstackgerrit Ed Leafe proposed openstack/nova master: Add a microversion for consumer generation https://review.openstack.org/562000
16:03:03 edleafe jaypipes: efried: cdent: ^^ now with microversion!
16:03:28 efried ack
16:03:43 edleafe jaypipes: efried: cdent: I used microversion 1.99 to avoid having to rebase as things merge. It will be changed to a real version once this gets close
16:04:04 bauzas arvindn05: okay, will try
16:04:06 efried edleafe: You'll still have to rebase, of course.
16:04:11 bauzas it was a busy day for me today
16:04:12 arvindn05 bauzas: ty
16:04:22 bauzas so hoping tomorrow will be different
16:04:35 bauzas for folks wondering, if you see me silent here, that means something else
16:05:35 openstackgerrit Merged openstack/nova stable/ocata: Handle volume-backed instances in IsolatedHostsFilter https://review.openstack.org/543635
16:05:42 edleafe efried: yeah, but just once more
16:05:43 openstackgerrit Merged openstack/nova stable/ocata: Fix docs for IsolatedHostsFilter https://review.openstack.org/543636
16:07:16 efried kashyap: You still around?
16:07:27 kashyap efried: Hi, yeah. But will step out in 10 minutes
16:07:34 kashyap Need to feed self
16:07:36 efried kashyap: Problem in your paste http://paste.openstack.org/show/719390/ is at L3
16:07:50 efried @mock.patch takes one positional arg.
16:08:16 kashyap efried: Yeah, I forgot to note that, I tried by removing the "return_value" from the @mock.patch
16:08:30 efried you get by that error (and on to the next :)
16:08:30 efried return_value='')
16:08:30 efried @mock.patch('nova.virt.libvirt.migration.get_updated_guest_xml',
16:08:30 efried kashyap: If you change it to
16:08:46 kashyap Right
16:08:49 efried if you wanted to mock.patch.object instead, then the first arg needs to be an object, the second a string naming a symbol.
16:09:07 kashyap I see
16:09:24 kashyap efried: I'm squashing a couple of commits that belong to the same logical change.
16:09:41 efried kashyap: Does this get you unwedged, or did I misunderstand what you were stuck on?
16:09:42 kashyap And then I'll duke it out tomorrow with the rest. I'll let you know
16:10:28 kashyap efried: A bit ago, I was tinkering by removing the return_value'' in the @mock.patch, and that got me somewhere
16:10:54 efried kashyap: The only thing you were probably accomplishing there was making the return value the name of the method you thought you were mocking.

Earlier   Later