| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2019-04-05 | |||
| 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 | i see several cases of this as well | |
| 15:11:30 | mriedem | guest = mock.Mock(spec='nova.virt.libvirt.guest.Guest') | |
| 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 | |
| 15:36:20 | sean-k-mooney | finucannot: mriedem this patch right https://review.openstack.org/#/c/635533/ | |
| 15:38:47 | finucannot | sean-k-mooney: yup | |
| 15:42:15 | sean-k-mooney | so it was not clear to me if https://bugs.launchpad.net/nova/+bug/1771506 is a release blocking bug for cannoical or not. i twould seam that xen would be broken due to this assueming that ubuntu/debian now shiped openssl 1.1.1 | |
| 15:42:16 | openstack | Launchpad bug 1771506 in OpenStack Compute (nova) "Unit test failure with OpenSSL 1.1.1" [Low,In progress] - Assigned to Corey Bryant (corey.bryant) | |
| 15:43:16 | openstackgerrit | sean mooney proposed openstack/nova master: SR-IOV Live migration indirect port support https://review.openstack.org/620115 | |
| 15:45:34 | sean-k-mooney | its a bit late but i think we should proably land https://review.openstack.org/#/c/635533/ or something similar soon and then figure out how to use the python libs for train | |
| 15:46:59 | sean-k-mooney | the final RC went out last night so this would not be in the GA release anyway right but i think it can be applied ot stable/* once we are happy with it | |
| 15:48:54 | finucannot | jaypipes: Thoughts on https://review.openstack.org/555081 this cycle? | |
| 15:49:15 | melwitt | cdent: thanks for the comment on the review. I had also asked (in the same stack of replies) if there's a way to detect whether we're running under wsgi or not? sean-k-mooney had suggested as a follow on patch, if we could choose between the executor type (eventlet vs not) based on wsgi/not it would be nice | |
| 15:49:27 | finucannot | jaypipes: I recall you were pretty fatigued with it last cycle. I'd be happy to pick it up if you're not keen to try again with it | |
| 15:50:29 | jaypipes | finucannot: go for it. | |
| 15:50:43 | finucannot | (y) | |
| 15:50:46 | jaypipes | finucannot: I'm pretty fatigued with most things OpenStack at this point. | |
| 15:50:47 | cdent | melwitt: hmmm. good question. I would think the wsgi `environ` will probably be able to tell you something, or sys.argv[0]? | |
| 15:50:55 | sean-k-mooney | melwitt: you can ask eventlet if things have been mokeypatch but we shoudl be able to provide a clearner way | |
| 15:51:08 | jaypipes | finucannot: and cdent's sighs and comments on that spec are spot on, fwiw. | |
| 15:51:21 | finucannot | agreed | |
| 15:51:43 | jaypipes | hardware-defined software strikes yet again. | |
| 15:51:44 | melwitt | cdent: ok, I will search in that direction. thanks | |
| 15:52:32 | cdent | melwitt: in environ there may be a way to get the name of the server, and the eventlet-driven server may have a unique name thus maybe a branch on that. goign the other way is harder since there are so many choices | |
| 15:52:44 | sean-k-mooney | melwitt: well what i was thinking is we proably can create the executor up front once and hide the decision form that function | |
| 15:53:28 | sean-k-mooney | melwitt: so in the case where we monkeypatch we can use teh greenlet on and where we dont we can use soemthing else | |
| 15:54:00 | melwitt | sean-k-mooney: doesn't the upfront thing have to choose too? I don't really understand what you mean | |
| 15:54:18 | cdent | there are two different entry points | |
| 15:54:27 | melwitt | oh, ok | |
| 15:54:58 | sean-k-mooney | yes that ^ and its cleaner to detect once then in all places that could need this in the future | |
| 15:55:00 | cdent | I can't remember the exact details, but the wsgi app has some unique code in advance of being-ready-to-serve | |
| 15:55:05 | melwitt | I see | |
| 15:55:12 | sean-k-mooney | assuming there are other asyc things we might want to do in the api like this | |
| 16:00:56 | mriedem | dansmith: you know how we restrict the request spec to the cell the instance is in before calling the scheduler during cold/live migrate and unshelve? | |
| 16:01:11 | mriedem | i do'nt see us doing that for evacuate, but seems we should... yeah? | |
| 16:01:37 | mriedem | you can't evacuate across cells | |
| 16:02:43 | dansmith | probably | |
| 16:03:17 | dansmith | I thought it was done in a place that was likely to catch any instance that already exists in the db | |
| 16:03:45 | mriedem | we explicitly lookup and set the cell on the Destination object in unshelve, cold and live migrate methods before calling the scheduler, | |
| 16:03:59 | mriedem | so it's not generic-ified anywhere as far as i know | |
| 16:04:24 | dansmith | hmm, okay it's been a long time since I looked, but okay | |
| 16:04:35 | dansmith | but yeah, if we're not we should for evac | |
| 16:04:59 | mriedem | cern must not do a lot of evacs or they are somehow otherwise restricting the server to a given cell with aggregates or something | |
| 16:05:01 | openstackgerrit | melanie witt proposed openstack/nova master: Use futurist.GreenThreadPoolExecutor in scatter_gather_cells https://review.openstack.org/650172 | |
| 16:05:09 | dansmith | they have tenant aggregates, | |
| 16:05:13 | dansmith | which probably fixes it for them, | |
| 16:05:20 | dansmith | s/,$// | |