Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2018-04-10
13:37:00 efried claudiub: Yeah, I recognize that; what I'm asking for in this case is a way to signal that I want to get around the decorator and autospec the actual method underneath it.
13:38:58 cdent efried: not really possible, your method has been redefined
13:39:24 efried cdent: Yeah, hence "black magic"
13:39:24 cdent the original form is sort of gone
13:39:41 cdent that would be darker than black
13:41:07 lpetrut efried: you may be able to retrieve the decorated methods, we do it in a few cases to avoid lock decorators within unit tests: https://github.com/openstack/os-win/blob/e0d7032dfb042f56fd02a52796b186cd8d67d240/os_win/_utils.py#L82
13:41:16 mriedem madhaviy: here is that ML thread http://lists.openstack.org/pipermail/openstack-dev/2017-September/121762.html
13:41:38 efried lpetrut: ooooo
13:42:50 cdent efried, lpetrut: too dark
13:43:03 efried ima try it anyway
13:43:08 efried cdent: hold my soul
13:43:23 cdent efried: wouldn't it be better to extract the thing you want to test to an undecorated thing?
13:44:05 efried cdent: I tried that first. Because it's already extracted thusly. But the decorator itself makes the test suite freak out. (It's the privsep entrypoint)
13:44:21 cdent it's all a bit smelly to me (not your soul (but maybe?))
13:44:36 cdent but we already know how I feel about complexity in tests...
13:44:44 efried I'm trying to soften the blows I keep on dishing out to mikal
13:45:06 efried cdent: FYI: https://review.openstack.org/#/c/552242/
13:47:40 mriedem madhaviy: there is also a thread in the operators ML with some other options
13:51:28 claudiub efried: yeah, as lpetrut said, we're getting the undecorated method is some unit tests in os-win, but that would only be needed for decorators which has some sort of special behaviour (adds / injects new arguments). even if we do autospec the undecorated methods, there are still plenty of cases in which the methods expects some sort of key-value argument, something like:
13:51:32 madhaviy mriedem: thanks. But I do not see any conclusion out of this discussion. Can we get back dhcp_domain conf option (not to deprecate)
13:51:33 claudiub if kwargs.get("something"): then do something
13:51:47 claudiub I've seen this in some oslo libs.
13:52:57 mriedem madhaviy: it definitely sounds like the option shouldn't be deprecated
13:53:09 mriedem at least until we know what we're going to do with it
13:53:52 mriedem i thought there was general agreement to use the dns_domain from the neutron network if there was one, and fallback to using the nova dhcp_domain option otherwise, but someone in the ops list punched some holes in that idea, but i'm not sure how valid those holes are, or if they are specific to that person's deployment
13:53:57 efried claudiub: Yeah, I understand that. That's not the case here, though - there are no arbitrary args. In this case, in the course of the review (over many iterations) I saw with my eyeballs that the invocation was wrong, and wanted a way to enforce it via autospec. But the decorator kills doing that directly. Now I'm hacking at it trying to figure out how I would actually use the undecorated method to do what I want.
13:54:28 mriedem madhaviy: if there were going to be a nova/neutron forum session at the summit, i'd put that in the agenda since ops would be in the room
13:54:32 mriedem melwitt: ^
13:55:05 madhaviy mriedem: sure.
13:55:15 claudiub efried: well, instead of autospec=True, you can pass-in a function that has the signature you want to enforce?
13:55:50 efried claudiub: As a side_effect to the mock, you mean?
13:56:11 claudiub something like: @mock.patch('something', autospec=some_function_with_exactly_what_I_want)
13:56:38 claudiub I dunno if that works though.
13:56:50 efried That sort of defeats the purpose, which is to vet whether my (actual) method is being called with the args I expect. If my test is wrong (in that it uses the wrong args) and then I make some_function_with_exactly_what_I_want have those same (wrong) args, then I'm just as effed.
13:59:22 claudiub yeah, a bit, but at least you'll have tests that don't have autospecs with *args / **kwargs in their method signatures.
14:00:29 mriedem madhaviy: melwitt: added https://etherpad.openstack.org/p/YVR-nova-brainstorming
14:00:31 mriedem mlavalle: ^
14:01:51 openstackgerrit Matt Riedemann proposed openstack/nova stable/queens: libvirt: Allow to specify granular CPU feature flags https://review.openstack.org/559700
14:02:04 mlavalle mriedem: cool, thanks!
14:02:41 mriedem dansmith: ^ rev'ed kashyap's backport if you want to hit that
14:03:07 kashyap mriedem: Ah, thanks. Was duking around fixing the compat code
14:03:13 kashyap (After removing the version stuff)
14:03:45 kashyap s/removing/bumping/
14:03:46 dansmith mriedem: done thanks
14:04:15 madhaviy mriedem: thanks!
14:05:29 kashyap mriedem: For the 'pike' change, abandoning it, and doing a fresh cherry-pick from 'queens' is the correct way?
14:05:42 mriedem kashyap: the change id is the same so you don't need to abandon
14:05:50 mriedem just re-do the cherry pick locally
14:05:53 kashyap Ah, right
14:07:13 openstackgerrit Matt Riedemann proposed openstack/nova stable/queens: libvirt: Allow to specify granular CPU feature flags https://review.openstack.org/559700
14:12:39 openstackgerrit Eric Berglund proposed openstack/nova master: PowerVM: Add proc_units_factor conf option https://review.openstack.org/554688
14:12:50 openstackgerrit Sylvain Bauza proposed openstack/nova-specs master: Proposes NUMA topology with RPs https://review.openstack.org/552924
14:15:32 mriedem seems the messages here should just say 'placement is required' right? https://github.com/openstack/nova/blob/6d4958b5c225a602d22069b920cbf1a37b652c58/nova/scheduler/client/report.py#L87
14:16:33 jaypipes mriedem: ack
14:19:42 kashyap Hmm, unrelated: "suddenly" I start seeing this import error when running unit tests: http://paste.openstack.org/show/718823/
14:19:43 mriedem ok i'll make that change. was looking at https://review.openstack.org/#/c/554920/ which will add a requirement to configure placement for nova-api,
14:19:57 mriedem so jaypipes - ^ will likely add the placement requirement before the mirroring patch does anyway
14:20:13 mriedem kashyap: tox -r -e
14:20:21 kashyap Darn, thanks
14:21:20 efried kashyap: If you want to not wait forever, you can start upgrading packages manually in your venv...
14:21:31 efried ...but -r is certainly safest.
14:22:10 jaypipes mriedem: ack
14:22:29 kashyap efried: Yeah, I hear ya. The manual imports might result in "find the next dep" rabbit hole. So just went with the "nuclear"
14:22:38 efried just so
14:34:07 openstackgerrit Matt Riedemann proposed openstack/nova master: Update wording in @safe_connect placement warnings https://review.openstack.org/560039
14:35:28 bauzas efried: jaypipes: stephenfin: sean-k-mooney: cfriesen_: thanks all for the reviews of https://review.openstack.org/#/c/552924/6/specs/rocky/approved/numa-topology-with-rps.rst
14:36:00 bauzas efried: jaypipes: stephenfin: sean-k-mooney: cfriesen_: now we have a new revision based on your comments https://review.openstack.org/#/c/552924/
14:36:04 openstackgerrit Merged openstack/nova master: Remove mox in unit/api/*/test_instance_actions.py https://review.openstack.org/559269
14:38:35 jaypipes bauzas: k, reviewing now
14:39:41 openstackgerrit Surya Seetharaman proposed openstack/nova master: [WIP] Delete orphaned req_spec/inst_mapp records of archived instances https://review.openstack.org/560042
14:55:58 openstackgerrit Chris Dent proposed openstack/nova master: Use nova.db.api directly https://review.openstack.org/543262
15:01:34 openstackgerrit Merged openstack/nova master: Remove mox in test_user_data.py https://review.openstack.org/559264
15:01:51 openstackgerrit Merged openstack/nova master: Remove mox in unit/api/*/test_server_metadata.py https://review.openstack.org/559673
15:02:02 openstackgerrit Merged openstack/nova master: Remove mox in unit/api/*/test_server_password.py https://review.openstack.org/559649
15:04:31 efried Spec cores (jaypipes dansmith because this spec is near and dear to your hearts) can we get https://review.openstack.org/#/c/556971/ approved now? Code is shaping up.
15:04:57 dansmith efried: wake me when jaypipes is +2 on it
15:05:02 efried ack
15:08:28 efried edleafe: You gonna rebase the rest of the consumer generation series to pick up that fix?
15:09:16 edleafe efried: already done (locally). Need to figure out one last bit before I push a new rev
15:09:22 efried coo
15:10:40 openstackgerrit Chris Dent proposed openstack/nova master: Use nova.db.api directly https://review.openstack.org/543262
15:18:50 jaypipes dansmith: I'm +2 on the spec now.
15:19:36 dansmith well, that was a short nap
15:20:03 mriedem lyarwood: at the ptg we talked about adding an online data migration to look for and remove duplicate bdm entries in the db, to eventually clear the way to adding a unique constraint on instance uuid and volume id in the bdm table - is that still on your radar, or something i can start hacking on?
15:20:29 mriedem L575 https://etherpad.openstack.org/p/nova-ptg-rocky
15:21:46 lyarwood mriedem: yeah, only just got around to starting yesterday when I hit https://bugs.launchpad.net/cinder/+bug/1762687
15:21:46 openstack Launchpad bug 1762687 in OpenStack Compute (nova) "Concurrent requests to attach the same non-multiattach volume to multiple instances can succeed" [Undecided,New] - Assigned to Lee Yarwood (lyarwood)
15:21:51 mriedem or was it a unique constraint on device name? now i'm confused
15:22:21 lyarwood mriedem: https://blueprints.launchpad.net/nova/+spec/remove-and-block-duplicate-bdms created that a while ago, wanted to catch up with melwitt or you about getting it approved etc this week
15:22:27 dansmith efried: the first bullet under "if there is no record" mentions stuff about user_id and project_id, but I'm missing why that's related
15:22:44 mriedem lyarwood: we don't need a blueprint for a bug fix
15:22:59 lyarwood mriedem: right, even if it's across two cycles?
15:23:00 mriedem lyarwood: as for that new concurrent requests bug, i know what that's about, and why it's only since queens
15:23:06 mriedem lyarwood: sure
15:23:12 lyarwood mriedem: kk, I'll nuke the bp then
15:24:15 efried edleafe: --^
15:24:58 efried dansmith: It's because of the goofiness we implemented wrt user and project IDs earlier, plus having no endpoints that manage consumers directly.
15:25:35 efried dansmith: At earlier microversions, proj/user IDs were optional, so we wanted to not create the consumer record if they weren't specified.
15:25:59 efried dansmith: But now we *always* want to set/maintain the generation, even at older microversions, so we *have* to create the consumer record.
15:26:25 efried dansmith: So we had to make proj/user ID fields nullable so that, at older microversions where they weren't required/specified, we could still create that record.

Earlier   Later