Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2019-09-16
14:18:28 efried stephenfin: waitwait
14:18:39 efried you're saying GET /a_c is called the appropriate number of times??
14:19:05 stephenfin That's what I'm seeing. Put the 'select_destinations.assert_called_once_with' to the end
14:19:13 stephenfin in select_destinations.assert_called_once_with
14:19:19 stephenfin sorry, test_select_destination_with_4_3_client
14:19:51 sean-k-mooney i have it open in the debuger now so ill check
14:19:52 stephenfin assert_called_once_with checks that things are called exactly once, right/
14:20:05 sean-k-mooney yes its called once
14:21:24 sean-k-mooney stephenfin: it check its called exactly once and has the correct args
14:21:28 sean-k-mooney so yes
14:21:53 efried I put a call_count assertion just in case
14:23:56 stephenfin moving the mocks inline and using context managers instead of function decorators doesn't help
14:24:47 sean-k-mooney if i put the select_destinations.assert_called_once_with( call at the end all the rest pass
14:27:43 sean-k-mooney its failing in assert_called_with when its comparing the args
14:32:46 stephenfin efried, sean-k-mooney: got it
14:32:50 efried tell
14:33:23 stephenfin https://review.opendev.org/#/c/671801/50/nova/scheduler/manager.py@191
14:33:33 stephenfin we're using extend, which modifies a list in place
14:33:52 stephenfin and our mock is returning 'fakes.ALLOC_REQS'
14:34:15 efried oy vay
14:34:17 stephenfin so that's getting modified by the non-disabled fallback test
14:34:40 efried stephenfin: I think I saw those globals only used by this one test suite?
14:34:53 stephenfin correct
14:35:11 efried so make 'em instance vars, for future safety
14:35:44 stephenfin wdym?
14:35:57 stephenfin I was doing to do 'fakes.ALLOC_REQS[:]'
14:36:10 stephenfin though I could add a 'get_fake_alloc_reqs' helper too
14:36:11 efried I mean, you could deepcopy 'em to fix this problem, but it's just going to bite us in the ass again later, somewhere else.
14:36:26 efried global test artifacts bad
14:36:32 stephenfin v bad.
14:36:43 efried first 205 lines of fakes, bad.
14:37:59 efried stephenfin: I guess for now you could just make it a helper method that returns a fresh new copy every time
14:38:08 efried but if it were me, I would make it completely fresh
14:38:27 efried def get_fake_alloc_reqs():
14:38:27 efried return { $everything }
14:38:55 efried rather than
14:38:55 efried EVIL_GLOBAL
14:38:55 efried def get_fake_alloc_reqs():
14:38:55 efried return some_unreliable_copy_method(EVIL_GLOBAL)
14:39:05 stephenfin gotcha
14:39:07 stephenfin coming right up
14:39:17 efried this is gonna be partway down the series, yah?
14:39:29 stephenfin yeah, just before that patch. I'll do it separately
14:39:57 efried stephenfin: you're going to have to fix the patch anyway, so might as well do it in place, nah?
14:40:09 efried don't try to fix the race afterward
14:40:39 stephenfin I meant add the helper function in a precursor patch and modify the intermittently failing patch to use it
14:40:42 efried tbc, the patch in question is "Add support for translating CPU policy extra specs, image meta"
14:40:42 stephenfin but I can combine too
14:41:03 efried yeah, just combine, help me justify a fast approve
14:41:08 stephenfin ack
14:47:38 openstackgerrit Stephen Finucane proposed openstack/nova master: Add support for translating CPU policy extra specs, image meta https://review.opendev.org/671801
14:47:39 openstackgerrit Stephen Finucane proposed openstack/nova master: fakelibvirt: Make 'Connection.getHostname' unique https://review.opendev.org/681060
14:47:39 openstackgerrit Stephen Finucane proposed openstack/nova master: libvirt: Mock 'libvirt_utils.file_open' properly https://review.opendev.org/681061
14:47:40 openstackgerrit Stephen Finucane proposed openstack/nova master: Add reshaper for PCPU https://review.opendev.org/674895
14:47:46 stephenfin efried: ^
14:48:06 stephenfin I'll remove the rest of those global fakes now (separate patch)
14:48:24 efried ++
14:48:47 openstackgerrit Matt Riedemann proposed openstack/nova master: Centralize volume create code during boot from volume https://review.opendev.org/682378
14:48:51 mriedem ^ is a simple refactor split off from https://review.opendev.org/#/c/541420/ which has been around since february of 2018,
14:49:06 mriedem and is important if y'all ever want to drop the legacy volume attachment compat code
14:50:57 efried stephenfin: +A, and re+W up the pile. Nice work, thank you.
14:59:24 stephenfin mriedem: done
15:00:13 efried mriedem: one adjustment requested pls
15:00:17 mriedem doing it
15:00:28 efried alex_xu: yt?
15:06:32 openstackgerrit Matt Riedemann proposed openstack/nova master: Centralize volume create code during boot from volume https://review.opendev.org/682378
15:06:32 openstackgerrit Matt Riedemann proposed openstack/nova master: WIP: Create volume attachment during boot from volume in compute https://review.opendev.org/541420
15:09:30 efried mriedem: are we merging stuff like that ^ at this point or waiting for ussuri to fork?
15:09:51 mriedem which one? the refactor is trivial and i've added the latter to https://etherpad.openstack.org/p/nova-train-release-todo
15:10:06 mriedem as i said, it's been around forever without much core review outside melwitt
15:10:12 mriedem the mox->mock stuff in the tests blew it all up
15:10:29 mriedem but if we ever want to migrate off the legacy volume attach code, we need to be creating all volumes with the new style attachment stuff
15:10:56 mriedem iow, the longer we wait, the bigger the data migration is going to be
15:11:10 mriedem e.g. https://review.opendev.org/#/c/549130/
15:11:40 mriedem i don't expect to get ^ into train at this point
15:11:46 mriedem nor is it probably the only way to skin that cat
15:12:29 mriedem at some point in the future we can add a nova-status upgrade check and fail to start if you haven't migrated old bdm records
15:12:50 mriedem and then drop all that compat code
15:15:44 mriedem melwitt: do you want to send this through? https://review.opendev.org/#/c/677736/
15:16:19 melwitt I do. thanks
15:26:06 openstackgerrit Balazs Gibizer proposed openstack/nova master: Follow up for the bandwidth series https://review.opendev.org/682389
15:26:29 gibi mriedem, efried: two smallish nits for the bandwidth series. I have two other testing enhancements on my TODO list that I plan to propose this week. None of these is critical, just nice to have.
15:27:17 mriedem gibi: i got the fup
15:27:37 gibi mriedem: thanks
15:27:55 gibi mriedem: and thanks for the review on the bug fix https://review.opendev.org/#/c/666857/ I will respin that
15:36:03 bauzas gibi: I can help you :)
15:56:29 gibi bauzas: thanks
17:26:10 openstackgerrit Eric Fried proposed openstack/nova master: objects: use all_things_equal from objects.base https://review.opendev.org/681397
17:39:31 openstackgerrit Merged openstack/nova master: Deprecate the XenAPIDriver https://review.opendev.org/680732
18:20:09 openstackgerrit Merged openstack/nova master: libvirt: Fix service-wide pauses caused by un-proxied libvirt calls https://review.opendev.org/677736
18:25:33 sean-k-mooney cool that ^ will make mdbooth happy
18:27:04 sean-k-mooney by the way can i get a second +2+w on https://review.opendev.org/#/c/670585/
18:27:39 sean-k-mooney i really want to ensure that is deprecated in train so we can remove it in Ussuri
18:27:52 sean-k-mooney as in like 2 weeks if not sooner
18:29:15 sean-k-mooney dansmith: mriedem: melwitt: could one of ye take a look when ye have time ^
20:03:02 mriedem sean-k-mooney: commented,
20:03:08 mriedem and added moshe and adrianc to it
20:04:22 sean-k-mooney thanks. if stephen does not adress the docs bugs ill do them tomorrow
20:05:26 sean-k-mooney mriedem: assuming no objection from moshe or adrianc are you generally ok with this?
20:06:41 mriedem i can't say i'm very familiar with the pci passthrough whitelist dev name stuff

Earlier   Later