Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2021-02-23
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
17:19:37 atmark will take a look
17:19:39 atmark thanks
17:19:49 prometheanfire now I have to check on fasteners (which I think needs a mask of 0.16 at least) and virtualenv (which is a whole... thing)
17:20:32 sean-k-mooney atmark: the old way is to use https://docs.openstack.org/nova/latest/admin/configuration/schedulers.html#aggregatemultitenancyisolation but you should use https://docs.openstack.org/nova/latest/admin/aggregates.html#tenant-isolation-with-placement if you can
17:20:50 sean-k-mooney atmark: i think the latter was added around rocky
17:21:07 openstackgerrit Stephen Finucane proposed openstack/nova master: tests: Remove ;test_tcp_rst_no_compute_rpcapi' https://review.opendev.org/c/openstack/nova/+/777168
17:21:12 stephenfin prometheanfire: ^
17:21:33 prometheanfire thanks
17:21:51 stephenfin gibi, melwitt, lyarwood, bauzas: That ^ is taken out of https://review.opendev.org/c/openstack/nova/+/714676 and should unblock mock 4.x in requirements. Could you take a look?
17:23:12 openstackgerrit Stephen Finucane proposed openstack/nova master: tests: Remove 'test_tcp_rst_no_compute_rpcapi' https://review.opendev.org/c/openstack/nova/+/777168
17:23:42 stephenfin aha, melwitt already has a better solution https://review.opendev.org/c/openstack/nova/+/741351/
17:25:08 stephenfin prometheanfire: https://review.opendev.org/c/openstack/nova/+/741351 is approved, so that should be fixed once it goes through the gate

Earlier   Later