Posted	Nick	Remark
#openstack-nova - 2019-03-06
20:19:17	efried	sean-k-mooney: I guess it "proves" (or at least suggests) that I didn't f up backward compatibility
20:19:27	efried	assuming of course that the func tests actually exercise the paths I dorked with.
20:19:56	sean-k-mooney	ya i guess.
20:21:02	sean-k-mooney	im just going to go grab some gorceries. ill be back online a bit later
20:22:00	efried	wow, grocery stores are open after 6pm? Things have changed since I lived over there:)
20:22:28	cdent	efried: the best part is that the sign says "open 24 hours"
20:22:36	cdent	but it means except saturday evening or sunday
20:23:03	efried	heh
20:23:21	cdent	I have anger about that because there was this one, not at band camp, when I needed to pee
20:23:49	cdent	and because I saw the "24 hours" I had let myself relax that little bit you do when you know that safety approaches
20:23:52	cdent	but no
20:25:30	efried	mriedem: Popping https://review.openstack.org/#/c/639854/ off the stack - did you say you were going to address both issues in a fup (and therefore consider this one done)?
20:26:11	efried	if you're loaded I can do it, no biggie
20:26:15	efried	(and by loaded I mean...)
20:27:15	mriedem	i can't hardly stand
20:27:46	mriedem	so i was going to start working on a fup patch for some of my comments in the earlier reshaper patches and then stack yours on top of that, but then ran out of steam to start doing that yesterday,
20:27:57	mriedem	and really kind of need to be working more on the cross-cell stuff
20:28:04	mriedem	so if you're looking for fun things to fup on, go nuts
20:29:46	efried	fup my nuts
20:33:46	openstackgerrit	Chris Friesen proposed openstack/nova master: Add missing libvirt exception during device detach https://review.openstack.org/641480
20:34:42	cfriesen	melwitt: this is for the bug you took a look at a few weeks ago. ^
20:34:43	mriedem	whoa
20:35:07	melwitt	cfriesen: ack
20:35:17	melwitt	on my list
20:42:33	openstackgerrit	Eric Fried proposed openstack/nova master: Test proper allocation of devices during reshape https://review.openstack.org/639854
20:42:45	efried	mriedem: There's that one with the implicit comma thingy removed.
20:42:58	efried	now fupping helper method for mdev name
20:44:46	mriedem	efried: did you see my comment about that earlier in the series?
20:45:02	mriedem	https://review.openstack.org/#/c/599208/18/nova/virt/libvirt/driver.py@6835
20:45:11	efried	yes
20:45:13	mriedem	i think we should just make _get_mediated_device_information handle the dash to underscore conversion
20:45:21	mriedem	so the caller (like your test) doesn't need to care
20:45:24	efried	ack
20:46:54	mriedem	do you want to just also fix those temporarily/PR typos in the test code in the same change as your test wrinkle or a new generic follow up?
20:46:58	efried	mriedem: one caller only has the name. Want the method to be smart and figure out which one it got, or take one of two kwargs, or...
20:47:35	mriedem	i don't follow
20:48:24	efried	_get_mediated_device_information is called in two places. In one of them, the one you pointed out, I have the UUID and convert to the name, which is the part that sucks. In the other, I only have the name and would have to do the reverse if I wanted the arg to be uuid.
20:48:25	mriedem	i guess you could add a helper method if that's more clear
20:49:06	mriedem	i figured at the start of _get_mediated_device_information it would just dev_name = dev_name.replace('-', '_')
20:49:10	mriedem	and then it doesn't matter
20:50:14	efried	the method uses both the name and the uuid. I think a pair of helpers might be clearest.
20:50:46	mriedem	oh you're talking about this right? https://review.openstack.org/#/c/599208/18/nova/virt/libvirt/driver.py@6161
20:51:16	mriedem	whatever, a simple _get_mdev_name(uuid) is also easy
21:01:35	openstackgerrit	Eric Fried proposed openstack/nova master: FUP for test_reshape https://review.openstack.org/641482
21:01:40	efried	mriedem: gathered them together ^
21:02:48	efried	'cept I seem to have f'ed that up somehow.
21:03:16	mriedem	dos2unix nice
21:04:06	efried	whaaa
21:04:33	efried	oh
21:04:47	efried	thought you were saying my line feeds were wrong
21:06:17	mriedem	so is this ok or are tests failing?
21:07:03	efried	failing. We must be monkey patching libvirt/utils somewhere
21:07:33	efried	f it, I'll move the utils into the driver.
21:07:35	mriedem	fakelibvirt...
21:07:57	mriedem	https://github.com/openstack/nova/blob/master/nova/tests/unit/virt/libvirt/fake_libvirt_utils.py
21:09:39	efried	that's... crazy
21:10:16	mikal	There's heaps of that stuff from back before we understood how unit testing worked.
21:10:32	mikal	Most of its removable with conversion to mock patches now.
21:10:43	mikal	(i.e. that's now been done for most of nova-net because I like doomed projects)
21:11:48	efried	clearly
21:12:14	mikal	*shrug* That code is 8 years old and no one has ever been offended enough to clean it up, so we're all complicit in it being like that.
21:12:17	openstackgerrit	Eric Fried proposed openstack/nova master: FUP for test_reshape https://review.openstack.org/641482
21:12:43	efried	oh ffs
21:12:44	mikal	It made sense at the time with the tools available then.
21:12:58	openstackgerrit	Eric Fried proposed openstack/nova master: FUP for test_reshape https://review.openstack.org/641482
21:13:07	efried	okay mriedem, hopefully that one is okay.
21:13:45	efried	I'm pretty offended mikal. But yeah, I don't know if I'm offended enough to clean it up.
21:15:13	mriedem	we're talking about fake_libvirt_utils being offensive right?
21:15:18	efried	yes
21:15:22	mriedem	not just mikal showing up and randomly offending people
21:15:26	efried	hahaha
21:15:41	dansmith	also,
21:15:47	dansmith	mikal is in his PJs currently, which may offend you
21:16:08	efried	for some values of "PJs".
21:16:10	dansmith	and for all we know, australian PJs might be like american birthday suits
21:16:18	efried	swhat I'm sayin
21:16:30	efried	...with a pouch
21:17:35	mikal	Look. I've only told one of you to fuck off so far today. I'm really not being _unusually_ offensive.
21:18:00	mriedem	i was going to say "as is his wont" but figured that might offend him, but i see it wouldn't
21:19:28	efried	We can hug it out, mikal, but let's get one thing perfectly clear:
21:19:31	efried	No pants, no hug.
21:19:52	mriedem	i'll hug a nude man
21:19:55	mriedem	i don't care
21:20:08	mriedem	i'm told it will be cold on the receiving end though
21:20:15	dansmith	last week mriedem told me he prefers sweaty guys
21:20:27	dansmith	I don't really like where this is going
21:20:59	mriedem	i'll go back to the fire i just started in the tc channel
21:21:09	dansmith	to warm your icy embrace?
21:28:44	melwitt	dansmith: going through the change to add a test case for an instance mapping that has no instance (manual cleanup or purge) and was thinking, what should we do in that case? if we don't migrate the user_id, the record would keep getting found. should we migrate it to some sentinel value? or bother trying to get it from request spec? or other?
21:31:50	cfriesen	are there docs on how to add new privsep commands in nova?
21:32:06	efried	cfriesen: You are right on time, mikal just put on pants
21:32:18	efried	if anyone has that answer, it's him.
21:32:36	mikal	Well, I actually _left_ to put on pants, but you get the idea.
21:32:55	mikal	cfriesen: do you want a pre-pants possibly grumpy answer, or a post pants probably still grumpy answer?
21:33:14	cfriesen	:)
21:34:11	cfriesen	I'm easy
21:34:11	mikal	cfriesen: https://www.madebymikal.com/adding-oslo-privsep-to-a-new-project-a-worked-example/ is an overly complete answer with way more detail than you care about.
21:34:38	mikal	cfriesen: but the short answer is its pretty trivial. Decide where in nova/privsep to put the method. Add the method with the decorator. Call the method from other places. Profit.
21:35:13	mikal	cfriesen: the only real warts are around style (don't pass in command lines, pass in args that are used to generate a command line); and unit testing hoops if efried notices that test coverage has dropped.
21:35:30	cfriesen	sweet, thanks.

Earlier   Later