| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2019-04-05 | |||
| 14:36:58 | dansmith | last I heard you weren't | |
| 14:37:00 | fried_rice | kashyap: That one shouldn't affect this at all. | |
| 14:37:05 | finucannot | I just switched that 'if not cells.enabled' check to 'if True:' so nova-net can never start now. It leaves reams of useless code but that's a trivial cleanup exercise now | |
| 14:37:15 | mriedem | dansmith: belmiro said in the ML that we could drop nova-net now too | |
| 14:37:18 | kashyap | fried_rice: Ah, this is a personal hack; nice | |
| 14:37:19 | dansmith | ah okay | |
| 14:37:26 | fried_rice | kashyap: But there was a flurry a few months ago where we changed the names of venvs and stuff. Might not have been in the specs repo though. | |
| 14:37:47 | fried_rice | kashyap: Yeah, looks like it boils down to the sphinx-build line with the right args, while sitting in the venv. | |
| 14:38:14 | mriedem | finucannot: if you're looking for something that needs to be done first but avoid breaking runtime code, fix the functional tests to not rely on nova-net by default https://github.com/openstack/nova/blob/master/nova/tests/functional/integrated_helpers.py#L82 | |
| 14:38:20 | openstackgerrit | Lee Yarwood proposed openstack/nova stable/queens: Delete instance_id_mappings record in instance_destroy https://review.openstack.org/650309 | |
| 14:38:29 | mriedem | https://github.com/openstack/nova/blob/master/nova/tests/functional/integrated_helpers.py#L87 | |
| 14:38:57 | mriedem | and get REQUIRES_LOCKING to default to False (it's True because by default we startup nova-network for functional tests) | |
| 14:39:37 | finucannot | mriedem: Sure, I can take a look at that. It's probably going to come up as I walk through this series fixing tests I've broken along the way | |
| 14:39:41 | finucannot | so. much. coupling. | |
| 14:40:10 | mriedem | finucannot: can you link me? | |
| 14:40:31 | finucannot | mriedem: to? | |
| 14:40:40 | mriedem | you said you had a patch series | |
| 14:40:47 | mriedem | but i don't see it in gerrit | |
| 14:41:07 | fried_rice | finucannot: better upload it before your computer crashes and you lose it. | |
| 14:41:38 | finucannot | mriedem: Nah, local only for now. It's failing from about half way in and I don't want to punish the gate (35 patches) | |
| 14:42:32 | mriedem | jesus | |
| 14:42:44 | mriedem | i really wish you would have brought this up before starting a huge local patch series | |
| 14:42:44 | finucannot | mriedem: https://github.com/stephenfin/nova/commits/cells-v1-removal | |
| 14:43:42 | finucannot | It's only a days work and I'll be folding a lot of that into other patches. If I end up dumping a lot of it, well, I learned stuff about the API | |
| 14:43:51 | finucannot | That's nowhere near final, btw | |
| 14:47:09 | kashyap | fried_rice: Thanks, noted. | |
| 14:47:11 | mriedem | ok so glancing through this you're killing of cells v1 separate from nova-net | |
| 14:47:18 | mriedem | starting with the api and cli entry points | |
| 14:47:22 | mriedem | and then working down | |
| 14:48:22 | finucannot | correct | |
| 14:48:39 | mriedem | if you can get what you have here cleaned up and passing, then i say push that up and let people start to look at it | |
| 14:48:55 | finucannot | Stop people using it so I can slowly break stuff without worrying about screwing up someones deployment | |
| 14:48:59 | finucannot | mriedem: ack | |
| 14:54:32 | mriedem | fried_rice: do we have a runways etherpad setup for stein yet and/or is anyone queueing for runways in stein? | |
| 14:58:58 | melwitt | easy review to fix the SynchronousThreadPoolExecutorFixture https://review.openstack.org/650171 | |
| 15:01:39 | mriedem | looking | |
| 15:02:49 | mriedem | melwitt: https://docs.python.org/3/library/unittest.mock.html#the-mock-class says | |
| 15:02:51 | mriedem | "spec: This can be either a list of strings or an existing object (a class or instance)" | |
| 15:03:12 | mriedem | maybe mock.Mock(spec='futurist.Future') was wrong b/c it wasn't a list? | |
| 15:03:30 | melwitt | oh, hm. I wonder why it doesn't work then. I've seen other examples in our code where a string doesn't work and attributes are not found | |
| 15:03:39 | melwitt | maybe. I can try that | |
| 15:04:09 | mriedem | i'm assuming you hit this while trying to de-eventlet some nova code or something? | |
| 15:04:19 | melwitt | haha yes | |
| 15:04:38 | mriedem | yeah it would be cool if we could use more of the native concurrency stuff... | |
| 15:05:10 | melwitt | yeah, I've got a patch going that I'm about to update again stacked on top of the fixture fix | |
| 15:07:27 | mriedem | looks like spec=[..] doesn't help | |
| 15:09:24 | fried_rice | mriedem: I haven't set anything up yet. I guess we have approved bps, so that should happen. I'll get on it. | |
| 15:10:21 | fried_rice | melwitt, mriedem: doesn't spec=futurist.Future work? | |
| 15:10:28 | mriedem | that's what she did | |
| 15:10:30 | melwitt | it does | |
| 15:10:31 | mriedem | and it does | |
| 15:10:32 | fried_rice | okay. | |
| 15:11:02 | melwitt | it's just the docs say you can use list of strings but it doesn't work when tried | |
| 15:11:10 | melwitt | so I dunno what's up with that | |
| 15:11:30 | mriedem | guest = mock.Mock(spec='nova.virt.libvirt.guest.Guest') | |
| 15:11:30 | mriedem | i see several cases of this as well | |
| 15:11:41 | mriedem | ntrn = mock.Mock(spec='neutronclient.v2_0.client.Client') | |
| 15:12:16 | melwitt | yeah, I have seen it throughout our code. and have forgotten every time to clean them all up to prevent more appearing | |
| 15:12:42 | melwitt | *help prevent | |
| 15:12:56 | openstackgerrit | melanie witt proposed openstack/nova master: Use futurist.GreenThreadPoolExecutor in scatter_gather_cells https://review.openstack.org/650172 | |
| 15:16:31 | fried_rice | claudiub might know more. | |
| 15:18:17 | fried_rice | melwitt, mriedem: IIUC, the list of strings is supposed to be a list of attributes the object is allowed to have. | |
| 15:18:28 | fried_rice | not, like, a list of classes. | |
| 15:18:30 | melwitt | haha, that would explain it | |
| 15:18:48 | fried_rice | so spec='bar' will actually make sure your mock has attributes 'b', 'a', and 'r' :) | |
| 15:18:53 | kashyap | fried_rice: (Thanks for the review here; just noticed: https://review.openstack.org/#/c/639091/ -- Rework 'EBUSY' (SIGKILL) error handling code path) | |
| 15:19:13 | melwitt | I'm reminded of the scene in zoolander when they try to get into the computer | |
| 15:19:36 | fried_rice | kashyap: Sure, fwiw :) | |
| 15:20:10 | kashyap | Yeah, it is one of those fixes that lowers the chance of "death-by-a-thousand-bugs" (which are the kinds of bugs I like to attack) | |
| 15:20:15 | kashyap | Instead of boiling ponds and lakes | |
| 15:20:18 | mriedem | looks like neither the xenproject ci (libvirt+xen) nor the xenapi ci have been running for a long time | |
| 15:20:36 | mriedem | finucannot: are you going to email them and copy the ML on ^? | |
| 15:20:49 | finucannot | mriedem: Yeah, I can do that | |
| 15:21:36 | fried_rice | melwitt, mriedem: Actually, with spec='foo', 'foo' is treated as an instance of str, so the mock gets all the attributes associated with string (index, replace, find, etc.) | |
| 15:22:08 | melwitt | ohhh man | |
| 15:22:35 | melwitt | haha | |
| 15:22:53 | fried_rice | what that *should* mean is that if you have a mock like m = mock.Mock(spec='nova.virt.libvirt.guest.Guest') and you try to reference e.g. m.uuid, it *should* blow up. | |
| 15:24:08 | fried_rice | melwitt: we should write a hacking rule | |
| 15:24:34 | melwitt | yeah, we should actually | |
| 15:24:42 | fried_rice | melwitt: Lemme throw one together right quick... | |
| 15:24:56 | fried_rice | ...but it means someone's gonna have to go through and scrub all of 'em. | |
| 15:25:18 | melwitt | I can do that. I had meant to do it in the past but kept forgetting | |
| 15:26:40 | finucannot | mriedem: Done, I think | |
| 15:26:56 | fried_rice | melwitt: Looks like there's only nine. | |
| 15:26:59 | fried_rice | stand by | |
| 15:28:04 | fried_rice | mriedem, melwitt: wanna merge this one real quick first so we don't have to rebase/renumber? https://review.openstack.org/#/c/649190/ | |
| 15:29:00 | finucannot | mriedem: Related. Could you give your take on this when possible? https://review.openstack.org/#/c/649974/ (ignore the failing pep8 test - I'll fix that shortly) | |
| 15:29:53 | mriedem | finucannot: i'm pretty sure there is already a patch related to that | |
| 15:30:10 | mriedem | which fried_rice is aware of | |
| 15:30:20 | mriedem | the xenapi + openssl warning one | |
| 15:30:51 | fried_rice | finucannot, mriedem: this one https://review.openstack.org/#/c/635533/ ? | |
| 15:31:06 | mriedem | yeah | |
| 15:32:21 | mriedem | maybe those aren't related i guess | |
| 15:32:24 | melwitt | fried_rice: looking | |
| 15:34:08 | finucannot | fried_rice, mriedem: Yeah, that looks like the same thing and is probably a better solution (though the tests are still rubbish, IMO) | |
| 15:34:40 | finucannot | well, similar thing. Solution for that should cover my issue | |
| 15:35:32 | jaypipes | cdent: your expertise needed on https://review.openstack.org/#/c/650172/3/nova/context.py please if you have a mo. | |
| 15:35:49 | jaypipes | thankie | |
| 15:35:55 | openstackgerrit | Stephen Finucane proposed openstack/python-novaclient stable/stein: Revert "Fix crashing console-log" https://review.openstack.org/650363 | |
| 15:36:04 | openstackgerrit | Stephen Finucane proposed openstack/python-novaclient stable/rocky: Revert "Fix crashing console-log" https://review.openstack.org/650364 | |