Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2026-08-25
12:32:48 gibi :)
13:06:05 opendevreview Balazs Gibizer proposed openstack/nova master: Stabilize TestInstanceNotificationSampleWithMultipleCompute https://review.opendev.org/c/openstack/nova/+/1002281
13:06:15 gibi another fix ^^
13:10:00 gibi I'm taking https://bugs.launchpad.net/nova/+bug/2164888 as well now
13:25:22 clif anyone familiar with a mypy failure around "ExceptionGroup" it complains the name is not defined despite it being a builtin...
13:31:39 clif also are nova tox functional tests able to be run locally?
13:31:53 sean-k-mooney an excpetion group is not a type
13:32:00 sean-k-mooney its a collection of types
13:32:19 sean-k-mooney as far as im aware at least
13:32:23 sean-k-mooney but no i ahve not seen that
13:32:34 clif wdym, it is a type isn't it? https://docs.python.org/3/library/exceptions.html#ExceptionGroup
13:33:02 sean-k-mooney hum ok added in 3.11
13:33:19 sean-k-mooney it wasnt a type before this is the first release we have been abot to use 3.11 features
13:33:38 sean-k-mooney before expction groups were jsut a tuple of possible expctions
13:34:18 clif it seems to work fine based on local testing but mypy blows up on it in tox pep8
13:34:48 sean-k-mooney why are you trying to define it in a type signigure?
13:35:10 sean-k-mooney 3.11 was for derive not the full feature
13:40:16 clif I'm not sure what you mean, I'm not using it as a type signature but as a raised exception due to stephenfin's review comment
13:40:23 sean-k-mooney i would not use it here for example https://review.opendev.org/c/openstack/nova/+/980679/12/nova/compute/manager.py#11572
13:40:37 sean-k-mooney which comment
13:41:03 clif https://review.opendev.org/c/openstack/nova/+/980679/comment/a6f02680_f3646440/
13:41:05 stephenfin clif: Got the review link to hand? I can take a look
13:41:09 stephenfin nvm :)
13:41:23 sean-k-mooney https://review.opendev.org/c/openstack/nova/+/980679/comment/a6f02680_f3646440/
13:41:37 sean-k-mooney so i dont think we shoudl be raiseing an execption groupu there
13:41:50 sean-k-mooney i intentlaly was raisign only the first excption
13:42:18 sean-k-mooney we coudl do that but we cant assuem the srounding code will handle the group properly
13:42:55 stephenfin clif: look at tool.mypy.python_version in pyproject.toml :(
13:43:04 clif stephenfin: aha :(
13:43:31 clif is it fine to bump that to 3.11?
13:43:33 stephenfin not the first itme I've been bitten by that https://github.com/python/mypy/issues/19349
13:43:51 sean-k-mooney ah 3.10 so it was a 3.11 related chagne
13:44:08 stephenfin clif: yeah, it should have been done when we bumped project.requires-python but obviously got missed
13:45:03 stephenfin just remember to run `tox -e mypy` locally before submitting since mypy _might_ raise new warnings about e.g. things that were newly deprecated in Python 3.11
13:45:14 stephenfin sean-k-mooney: ExceptionGroup is new in 3.11, yup
13:45:16 sean-k-mooney will an expction group match an excpt block that only matches on the encloded exeptions?
13:45:28 sean-k-mooney and will it properly handel all of them or just one
13:45:41 stephenfin I believe so, but there's a very easy way to test it ;)
13:45:44 opendevreview Clif Houck proposed openstack/nova master: Bump python_version for mypy to 3.11 https://review.opendev.org/c/openstack/nova/+/1002290
13:46:04 sean-k-mooney stephenfin: right so the issue i have is your chanign the behvior of the algorthim not just changing the typing
13:46:17 sean-k-mooney again it was intentlly only rasing the first excption not all of them
13:46:36 sean-k-mooney it was loging all of them
13:49:34 stephenfin Sorry, what algorithm was changed?
13:50:56 sean-k-mooney it now raising all the expctions as a group instead of just the first one
13:51:08 sean-k-mooney in the non paralle mode it would have failed after the first expction
13:51:21 sean-k-mooney so the intent was to keep the external behvior the same
13:51:36 sean-k-mooney we can rasie all of them as a group but that a slightly diffenernt semantic
13:52:19 stephenfin (a) that desire isn't captured anywhere so I'm not sure if that was the intent
13:52:34 sean-k-mooney that was part of the code i wote
13:52:42 sean-k-mooney before clif took it over so that was the intent
13:52:46 stephenfin and (b) the behavior has already changed since in the non-parallel mode, only N/M operations would have run (where the Nth operation raised the exception) whereas now M operations will run
13:54:35 stephenfin if operation N raises FooException and N+X raises BarException, raising only FooException is lossy and makes things harder to debug. My suggesion was to try to avoid that
13:54:44 sean-k-mooney both can work but im not sure what we get by raisign all of them
13:55:14 sean-k-mooney stephenfin: that why all of them were logged
13:55:16 sean-k-mooney for debugging
13:55:29 sean-k-mooney stephenfin: we intend to backport this
13:55:40 sean-k-mooney so that why im qustioning using a 3.11 feature
13:55:52 stephenfin okay, backporting does change the equation. I wasn't aware of that
13:55:58 sean-k-mooney well we intned to backprot he prevsiou patch this wone was deabateable
13:57:58 sean-k-mooney we can modify it on backport if requried but ya the hope ofr the O(n2) patch at least was to backport to all stable branches
13:58:23 sean-k-mooney the parallisze i know operator will want it backported but im not sure it fully fits stabel policy
13:58:29 stephenfin okay, if that's the plan then yeah, we'd want to go back to just raising the first exception
13:58:31 sean-k-mooney so that patch may be propsoedn abut not mergd
13:59:23 sean-k-mooney *may be proposed but not merged
13:59:48 sean-k-mooney we can do both we could add a patch to swap to excption groups as a followup
14:00:02 stephenfin clif: ☝️ apologies for the slight waste of your time there 🙇
14:00:04 sean-k-mooney but if we can keep this 3.10 compatiable it woudl be nice
14:03:41 sean-k-mooney stephenfin: since your about https://review.opendev.org/c/openstack/nova/+/1002246 gibi puhsed htat to stablise a functional test mind taking a look
14:11:14 clif ok, yea I can revert the ExceptionGroup change
14:12:28 clif relatedly, some functional tests are failing on the parallel per-node resource update change, for instance: nova.tests.functional.libvirt.test_pci_in_placement.PlacementPCIInventoryReportingTests.test_device_reconfiguration_with_allocations_config_change_stop fails because the exception raised has changed
14:14:58 clif any advice on that?
14:19:22 clif well now that I think about it some the failures may be related to the exceptiongroup change
14:34:48 clif ah yea it was the exceptiongroup changing how exceptions got raised, makes sense
14:39:23 opendevreview Clif Houck proposed openstack/nova master: Parallelize per-node resource updates https://review.opendev.org/c/openstack/nova/+/980679
15:05:10 gmaan gibi: ack, will check
15:19:58 gibi gmaan: thanks
15:23:11 gmaan I saw this again, I think this hit twice, I will work on this today https://bugs.launchpad.net/nova/+bug/2164880
15:26:37 gibi gmaan: thanks
15:29:09 melwitt thank you gmaan for working on these
15:31:10 melwitt sean-k-mooney: gotcha, I see. yeah. really hope we can switch to sdk rather than add those one-off ... the patch is not too ugly (I hope!)
16:56:18 stephenfin melwitt: I'm +2 on https://review.opendev.org/c/openstack/nova/+/1002007 again. The ksa fix is also merged and should be released this week, so we could potentially remove that subclass next week. Just need sean-k-mooney to revisit that now (hint hint)
17:00:02 sean-k-mooney[m] cool we are at the park with frya ill be home in a bit and ill take a look then
17:22:59 melwitt stephenfin: super, thank you
17:28:58 opendevreview Stephen Finucane proposed openstack/nova master: cinder: Use keystoneauth1 Adapter for endpoint discovery https://review.opendev.org/c/openstack/nova/+/1002359
18:07:42 sean-k-mooney stephenfin: melwitt im pretty sure that https://review.opendev.org/c/openstack/nova/+/1002007/8/nova/accelerator/cyborg.py is incorrect
18:07:56 sean-k-mooney stephenfin: melwitt we do not use an admin client today to talk to cyrbog
18:08:04 sean-k-mooney wew shoudl in the future but we dont today
18:08:18 sean-k-mooney and that cached client is confging it ot an admin client
18:08:45 sean-k-mooney the cachign itself might makes sense in the future but not today when we alwasy need to use the users token
18:09:54 sean-k-mooney i think the problem is that this is folow the same pattern we use to tak to ironic and manilla but the cybrog interaction is not the same as the inronic interaction
18:09:55 melwitt sean-k-mooney: hm ok, so that should be admin=False. I had thought it was but that must have been an earlier iteration
18:10:43 sean-k-mooney ya which means we take the if brnach here https://review.opendev.org/c/openstack/nova/+/1002007/8/nova/utils.py#940
18:10:45 sean-k-mooney not the else
18:11:04 sean-k-mooney but if we take the if brnach we done need to chagne the signigure to pass in ksa_auth
18:12:06 sean-k-mooney if admin false branch already correctly constuct the service user token and uses the uers token form the context to constuct the sdk connection
18:12:23 melwitt argh dammit
18:13:01 melwitt sorry
18:13:42 sean-k-mooney on a side note i messed up my grub config on one of my dev hosts, i do not know the root password adn signle user mode didnt work so im not reinstalling an os
18:13:47 sean-k-mooney so its one of those days
18:14:15 melwitt I have been there

Earlier   Later