| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2019-04-05 | |||
| 13:41:35 | finucannot | sean-k-mooney: and it'll be totally separate since this series already weighs in at 34 patches | |
| 13:41:44 | finucannot | Some of which I'll be able to fold in but still | |
| 13:42:26 | sean-k-mooney | ya it should be seperate | |
| 14:16:59 | fried_rice | kaisers: looking... | |
| 14:17:12 | fried_rice | finucannot: Specless BP for cellsv1 removal, hells yeah. | |
| 14:23:14 | dansmith | I think we need a real spec for cellsv1 removal | |
| 14:23:40 | melwitt | finucannot: +1 on removing cells v1 and nova-net separately | |
| 14:28:46 | mriedem | we have had specless bps in the past for nova-net removal | |
| 14:28:58 | fried_rice | kaisers: +A, but a couple of questions inline | |
| 14:29:20 | mriedem | https://blueprints.launchpad.net/nova/+spec/remove-nova-network | |
| 14:29:24 | fried_rice | dansmith: what would the spec say? | |
| 14:29:42 | dansmith | fried_rice: I was totally joking, but you people are slow and the moment was ruined | |
| 14:29:54 | mriedem | note nothing happened in stein https://blueprints.launchpad.net/nova/+spec/remove-nova-network-stein | |
| 14:29:58 | fried_rice | oy | |
| 14:30:40 | mriedem | also note it's not as easy as just deleting some code | |
| 14:30:54 | mriedem | there are still APIs that are nova-net specific that need to be changed to 410s https://etherpad.openstack.org/p/nova-network-removal-rocky | |
| 14:31:42 | finucannot | mriedem: Oh, I know | |
| 14:31:47 | finucannot | that's the first patch in my series | |
| 14:31:57 | finucannot | Sorry, I meant for cells | |
| 14:32:03 | finucannot | same thing for nova-net tho | |
| 14:32:35 | mriedem | i also tend to think the people that are familiar with this code should be doing the removal - i didn't realize you'd already started a removal series | |
| 14:32:54 | kashyap | Hey folks, is there some shortcut to build just _one_ releases's specs? E.g. `tox -e docs train` or something like that | |
| 14:33:10 | mriedem | kashyap: no | |
| 14:33:29 | kashyap | Okido; figured as much | |
| 14:34:13 | finucannot | mriedem: I figured the important bit was that _somebody_ did it | |
| 14:34:25 | fried_rice | kashyap: yes, hold | |
| 14:34:26 | finucannot | Plus, nothing stops those people review the series and provide their insight that way | |
| 14:35:14 | mriedem | finucannot: at the very least i think there needs to be a plan | |
| 14:35:17 | fried_rice | kashyap: http://paste.openstack.org/show/748945/ | |
| 14:35:46 | mriedem | finucannot: like, do you know about the nova-net and cells v1 dependency for startup? | |
| 14:35:52 | fried_rice | kashyap: Haven't actually used it in a while, it's possible some of the path assumptions will need to be tweaked since we've been dorking with tox.ini lately. | |
| 14:35:53 | finucannot | mriedem: That's fair. PTG is only a few weeks away. I drafted this in a day so I'm happy to let it sit in Gerrit til then | |
| 14:35:56 | mriedem | and if so, how are you planning on removing the stuff based on deps? | |
| 14:36:06 | finucannot | mriedem: that nova-net won't start unless cells v1 is enabled? | |
| 14:36:11 | kashyap | fried_rice: Ah, thanks. /me clicks | |
| 14:36:31 | mriedem | right - so going to remove nova-net first? which is harder, or break that dependency and remove cells v1 first | |
| 14:36:46 | kashyap | fried_rice: Yeah, I saw your tox.ini patch fly-by for the 'specs/backlog' thingie | |
| 14:36:53 | dansmith | tssurya: you're done with v1, but are you done with n-net? | |
| 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. | |