| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2018-04-10 | |||
| 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 | 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:46 | lyarwood | mriedem: yeah, only just got around to starting yesterday when I hit https://bugs.launchpad.net/cinder/+bug/1762687 | |
| 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. | |
| 15:26:34 | efried | I think I've got that right - edleafe help me out here ^ | |
| 15:27:26 | dansmith | hmm, okay, that is.. odd, | |
| 15:27:31 | efried | it is indeed. | |
| 15:27:37 | dansmith | so we didn't initially have a consumer record and then added it for user/proj? | |
| 15:27:47 | efried | I think that's the case, yes. | |
| 15:27:50 | dansmith | I guess it seems weird that we didn't just start creating those records with null fields at that point | |
| 15:28:00 | efried | yeah. That would have been a thing to do. | |
| 15:28:02 | efried | Somewhere I tagged the IRC discussion edleafe and I had about this. | |
| 15:28:05 | 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:28:05 | mriedem | lyarwood: details https://bugs.launchpad.net/nova/+bug/1762687/comments/3 | |
| 15:28:08 | efried | possibly in a spec comment. | |
| 15:28:25 | dansmith | efried: so ... is it unreasonable to say that expecting the reader of the spec to have that context is.. unreasonable? | |
| 15:28:34 | efried | dansmith: Here's that IRC convo: http://eavesdrop.openstack.org/irclogs/%23openstack-nova/%23openstack-nova.2018-04-04.log.html#t2018-04-04T13:35:30 | |
| 15:28:36 | dansmith | because reading it from the top, I get stopped at that point with zero idea where it's going | |
| 15:29:09 | efried | dansmith: That's not unreasonable. But trying to explain all that context probably would be. How about a vague "for historical reasons" sentence? | |
| 15:29:23 | efried | or I suppose we could link that eavesdrop in | |
| 15:29:53 | dansmith | well, | |
| 15:30:01 | dansmith | I kinda think the context is worthwhile in here | |
| 15:30:04 | dansmith | more than an irc link | |
| 15:30:38 | efried | Or perhaps this is an implementation detail that doesn't need to be in the spec at all. Since I *think* the interface isn't changing. (edleafe said it was changing a teeny bit; I never understood how) | |
| 15:30:43 | lyarwood | mriedem: I think you've missed that this is with two different instances, not one. | |
| 15:31:39 | mriedem | oh.... | |
| 15:31:42 | mriedem | yes, dear | |
| 15:35:13 | dansmith | efried: edleafe: comments in there.. if you're really concernedand want to do those in a follow-in that's fine, but I think it's probably fine to just roll them in here | |
| 15:35:23 | dansmith | and I can fast approve if jaybird isn't around at that point | |
| 15:36:04 | kashyap | When anyone gets a moment later, I'm duking around a potentially stupid unit test mistake: http://paste.openstack.org/show/718840/. Corrections / snide remarks / rotten tomatoes welcome. | |
| 15:36:13 | kashyap | s/bbia/bbiab/ | |
| 15:36:57 | openstackgerrit | Surya Seetharaman proposed openstack/nova master: Cleanup RP and HM records while deleting a compute service. https://review.openstack.org/554920 | |
| 15:41:40 | mriedem | lyarwood: i don't have great solutions off the top of my head | |