| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2021-02-23 | |||
| 16:16:57 | gibi | nvm I see it | |
| 16:17:09 | stephenfin | why or where? | |
| 16:17:10 | gibi | so in the past there was a case when the guest was fully started | |
| 16:17:16 | gibi | and then we failed | |
| 16:17:19 | gibi | so we need to stop is | |
| 16:17:20 | gibi | it | |
| 16:17:37 | gibi | but now it either starts without exception or fails before it is started | |
| 16:17:37 | stephenfin | if the firewall driver couldn't apply filters, right? | |
| 16:17:42 | gibi | yepp | |
| 16:17:49 | stephenfin | yeah, that's my reading of this too | |
| 16:18:04 | stephenfin | I did the firewall cleanup too but missed that. Apologies | |
| 16:18:53 | gibi | hm, I don't know what happens if create_guest fails at libvirt_secret.undefine() in the finally block, at that point we have a running guest I assume | |
| 16:19:20 | gibi | but we don't get back the guest object from the caller | |
| 16:19:25 | gibi | to stop it | |
| 16:19:59 | stephenfin | yeah, an exception will still be raised | |
| 16:20:44 | stephenfin | the finally block just executes stuff before the finally happens, iirc | |
| 16:20:52 | stephenfin | akin to save_and_reraise | |
| 16:22:58 | gibi | so we have to catch possible exceptions from libvirt_secret.undefine(), LOG them, and still return the geust object | |
| 16:23:07 | gibi | guest | |
| 16:23:13 | stephenfin | apparently not | |
| 16:23:13 | stephenfin | https://docs.python.org/3/reference/compound_stmts.html#finally | |
| 16:23:22 | stephenfin | If finally is present, it specifies a ‘cleanup’ handler. The try clause is executed, including any except and else clauses. If an exception occurs in any of the clauses and is not handled, the exception is temporarily saved. The finally clause is executed. If there is a saved exception it is re-raised at the end of the finally clause. If the finally clause raises another exception, the saved exception is set as the context of | |
| 16:23:22 | stephenfin | the new exception. If the finally clause executes a return, break or continue statement, the saved exception is discarded: | |
| 16:23:32 | stephenfin | Sorry | |
| 16:23:32 | stephenfin | If the finally clause raises another exception, the saved exception is set as the context of the new exception. | |
| 16:23:49 | stephenfin | so it'll basically do raise undefine_exception from other_exception | |
| 16:24:15 | gibi | yeah that is how I understand | |
| 16:24:48 | stephenfin | yeah http://paste.openstack.org/show/802935/ | |
| 16:25:49 | gibi | if we let the exception propagate from the finally block then we will leave a running guest behind | |
| 16:25:59 | gibi | and the caller cannot stop it | |
| 16:26:08 | gibi | as it has no reference to that guest object | |
| 16:26:26 | stephenfin | Hmm, good point | |
| 16:26:41 | stephenfin | so if the undefine call fails | |
| 16:27:24 | stephenfin | we won't have a running guest before that since launching it is the last thing we do (returning won't cause an exception) | |
| 16:28:47 | gibi | you are right sorry | |
| 16:29:10 | gibi | so no need to change here | |
| 16:33:49 | openstackgerrit | Balazs Gibizer proposed openstack/nova master: Follow up type hints for a634103 https://review.opendev.org/c/openstack/nova/+/777159 | |
| 16:44:29 | bauzas | woah, the gate pipeline is late | |
| 16:54:24 | gibi | maybe it is already the churn before the feature freeze | |
| 16:55:11 | stephenfin | bauzas: There are issues. They were discussing it on #openstack-infra earlier | |
| 16:55:19 | stephenfin | Still are, maybe | |
| 16:55:23 | bauzas | kk | |
| 16:55:27 | bauzas | thanks | |
| 17:03:44 | prometheanfire | we're coming up on the end of the release cycle and nova still hasn't fixed mock :| | |
| 17:05:21 | sean-k-mooney | prometheanfire: its not really broken is it. | |
| 17:05:43 | stephenfin | prometheanfire: I thought we'd said that we didn't need to? | |
| 17:06:11 | stephenfin | mock (the third party lib) has fixes that weren't backported to python3.6 | |
| 17:06:43 | stephenfin | and I'm pretty sure the reason mock was capped at <4.0 (iirc) was because that had its own bug | |
| 17:07:26 | stephenfin | prometheanfire: Can you remind me of the reason nova had to switch? I'd stopped focusing on it because working around python3.6 unittest.mock bugs seemed wrong | |
| 17:07:56 | sean-k-mooney | wel should eventulaly swicht to not useing mock the lib at all | |
| 17:08:02 | sean-k-mooney | and use the standard lib only | |
| 17:08:03 | stephenfin | sean-k-mooney: why | |
| 17:08:14 | sean-k-mooney | because mock the lib has some bugs | |
| 17:08:23 | sean-k-mooney | it does not fully work the same as the stanard lib | |
| 17:08:35 | stephenfin | See above | |
| 17:08:41 | prometheanfire | requirements doesn't cap mock | |
| 17:08:48 | sean-k-mooney | and since we are pythoh 3 only now we dont need it any more | |
| 17:08:55 | stephenfin | To the best of my knowledge, it's a carbon copy of unittest.mock from the latest stable python | |
| 17:09:10 | stephenfin | including all the new features and bugfixes found therein | |
| 17:09:18 | prometheanfire | if the mock-4 releases are bad they should be masked so they don't bug me every day when the constraints bot review fails because nova doesn't work with the new mock (but other projects aparently do) | |
| 17:10:03 | stephenfin | prometheanfire: I'm almost certain we're seeing the exact same issues with unittest.mock on either python3.9 or python3.10 | |
| 17:10:07 | sean-k-mooney | stephenfin: there was deffinlty differnt behavor in the past as code in neutorn would not work in nova | |
| 17:10:32 | stephenfin | and there's an open bug against it but it hasn't gone anywhere | |
| 17:11:02 | stephenfin | prometheanfire: Have you link to a failing constraints bot job? | |
| 17:11:05 | stephenfin | *a link | |
| 17:11:07 | prometheanfire | yep | |
| 17:11:17 | prometheanfire | https://review.opendev.org/776967 | |
| 17:11:25 | stephenfin | ta | |
| 17:12:00 | prometheanfire | was the previous work abandoned? switching to unittest.mock iirc | |
| 17:12:26 | stephenfin | it's not abandoned in Gerrit terms, but nobody is looking at it | |
| 17:12:50 | stephenfin | https://review.opendev.org/c/openstack/nova/+/714676 | |
| 17:12:50 | sean-k-mooney | this is becasue of eventlets and py39 | |
| 17:12:55 | prometheanfire | which is why I pinged :P | |
| 17:12:59 | sean-k-mooney | the websocket proxy is broken on py39 | |
| 17:13:06 | sean-k-mooney | because of dnspython | |
| 17:13:13 | prometheanfire | ya, turtles | |
| 17:13:37 | sean-k-mooney | well actull its brok on 36 too | |
| 17:13:52 | sean-k-mooney | eventlest does not supprot dnspython 2.0 | |
| 17:14:09 | stephenfin | prometheanfire: Yes, look https://review.opendev.org/c/openstack/nova/+/714676/9/nova/tests/unit/console/test_websocketproxy.py | |
| 17:14:10 | prometheanfire | only worked by accident on 3.7 and 3.8? | |
| 17:14:12 | sean-k-mooney | that websocket issue look kindo fo like the double wrappign issue | |
| 17:14:46 | prometheanfire | ya, magicmock | |
| 17:14:52 | stephenfin | prometheanfire: To get it working with stdlib mock, we had to remove the same test that's failing there | |
| 17:15:07 | stephenfin | prometheanfire: We can push a patch to master to remove that test and unblock requirements | |
| 17:15:10 | stephenfin | I'll do that now | |
| 17:15:17 | sean-k-mooney | stephenfin: yep this is one of the case i think where there is a delta between standard mock and mock the lib | |
| 17:15:25 | prometheanfire | if you don't mind | |
| 17:15:36 | stephenfin | sean-k-mooney: I'm saying unittest.mock and mock the lib are having the exact same issue :) | |
| 17:15:40 | stephenfin | because they're the same thing | |
| 17:16:13 | prometheanfire | and using mock as a lib is fine, it is one MOVING target for all python versions, vs a locked target per version of supported python | |
| 17:16:24 | prometheanfire | pick your poison kinda | |
| 17:16:38 | sean-k-mooney | maybe now but again on python 2.7 it was not | |
| 17:16:51 | sean-k-mooney | anyway we shoudl proably revie https://review.opendev.org/c/openstack/nova/+/714676 | |
| 17:17:17 | sean-k-mooney | but delete the current test i guess works for now | |
| 17:17:34 | atmark | is there filter can tag a set of computes to a tenant? | |
| 17:17:56 | stephenfin | prometheanfire: Yeah, I was all for dropping it in e.g. oslo projects, but we obviously have way more tests than anything there and when you start looking at the amount of bugs we need to workaround on older stdlib versions it seems less attractive | |
| 17:18:07 | sean-k-mooney | atmark: yes | |
| 17:18:38 | stephenfin | We'll do it _eventually_, but it's dropped down my priority list rn (and melwitt's too, evidently) | |
| 17:18:40 | prometheanfire | stephenfin: yep | |
| 17:18:42 | sean-k-mooney | there is both a post fitler and prefilter https://github.com/openstack/nova/commit/732e202e81142a8ea462a9ebcde9a7226a62a60b | |