Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2025-05-08
15:24:28 sean-k-mooney i belive i had that in my verison ot prevent leakign thread betwen tests
15:24:39 sean-k-mooney if we ever do leak a thread in the pool it should fail the test
15:24:55 sean-k-mooney the same way our greenthread leak proctection works
15:26:14 sean-k-mooney in the test we can kind of get awar with it by using the synconousthreadpool via a fixture too
15:27:34 sean-k-mooney but we do have fixture to mange this currently https://github.com/openstack/nova/blob/master/nova/test.py#L197 https://github.com/openstack/nova/blob/master/nova/test.py#L324-L329
15:27:55 sean-k-mooney so we likely need to extend this to the new scather gather pool
15:29:19 sean-k-mooney i made the SynchronousThreadPoolExecutorFixture also apply to the futurist.ThreadPoolExecutor https://review.opendev.org/c/openstack/nova/+/922497/5/nova/tests/fixtures/nova.py#1254
15:33:10 sean-k-mooney gibi: bug i think you shoudl also update https://github.com/openstack/nova/blob/master/nova/tests/fixtures/nova.py#L1191-L1247 and https://github.com/openstack/nova/blob/master/nova/tests/fixtures/nova.py#L2056 or add new ones for this thread pool
16:59:55 gmaan sean-k-mooney: RE on group_policy, I think namespace is good idea, not sure why it was left in initial effort.
17:01:05 gmaan sean-k-mooney: about changing default to None. is not it None as default in ResourceRequest object and as you mentioned it is set 'None' explicitly if two group from different than flavor source
17:01:47 gmaan I mean just checking what is value if it is not passed and groups from cmg from flavor only
17:02:50 sean-k-mooney gmaan: right i know we set it if we get groups form 2 souces
17:03:06 sean-k-mooney im jsut suggesting updating that logic ot set it if we ever have 2 groups and its not set
17:03:23 sean-k-mooney so if you set it in the flavor that woudl alays overwrite it
17:04:06 gmaan sean-k-mooney: yeah, I agree on that. I am just saying we might be doing the same currently (not in flavor but somewhere in request spec/ResourceRequest )?
17:04:56 sean-k-mooney gmaan: not based on gorkas testing of tryign to use 2 groups to ask for two vgpus
17:05:23 sean-k-mooney but maybe there are some code paths where we will
17:05:47 gmaan humm, and is it set 'isolated' in that case?
17:05:56 sean-k-mooney no its not pass to placement at all
17:06:03 gmaan ohk
17:06:03 sean-k-mooney and then placement raises an error i belive
17:06:23 sean-k-mooney i can ask for some logs or access to there enve where they hit this
17:06:35 sean-k-mooney we likely can create a repoducer
17:07:46 gmaan I think I agree with your proposal. As 'none' means request can be satisfied by different or same providers. if not passed, setting it default to None in flavor make sense. I do not think it change behavior from user perspective.
17:08:24 gmaan we are just adding default here and not actually *changing* the default
17:08:44 sean-k-mooney correct that is what it means
17:09:16 sean-k-mooney gmaan: at least based on what we have documetned here https://docs.openstack.org/nova/latest/user/flavors.html
17:09:52 gmaan yeah, I do not think we need microversion in that case, we can fix it as bug fix
17:10:47 opendevreview Ishan Shanware proposed openstack/nova master: [FEAT]: using quota_details to optimize validate networks [FIX]: added error handling for .get() statements [FIX]: pep8 formatting fixed for changes [FIX]: added additional unit tests changes to mock validate_networks [FIX]: fixes using autopep8 https://review.opendev.org/c/openstack/nova/+/949235
17:10:57 sean-k-mooney thansk for confirming. as i said above i didnt like the idea of a server create failign with a given flavor based on the microversion used
17:10:59 sean-k-mooney or resize
17:11:15 sean-k-mooney that just seam very confussing for the end user to reason about
17:12:43 gmaan yeah
17:13:20 sean-k-mooney gmaan: there si already a bug for the filter part https://bugs.launchpad.net/nova/+bug/2048874 so ill likely resue that
17:14:27 sean-k-mooney ill proably just take over the orgianl authors patch and try an dalign it to what we have dicussed https://review.opendev.org/c/openstack/nova/+/905210
17:14:57 sean-k-mooney they were orginall just oging to skip that key which may have worked but its nto really fixign the problem
17:15:45 sean-k-mooney although i might just reuse the bug and not the patch since the logic of the fix will end up beign differnt
17:17:25 sean-k-mooney while i have this open https://review.opendev.org/c/openstack/nova/+/947541 is a nice small fix form melwitt that might be nice ot land if folks agree
17:19:26 melwitt sean-k-mooney: thanks for reviewing that :) I am happy to respin with code comment + release note btw
17:20:00 sean-k-mooney frickler: fungi: do either of ye op access on the channel it might be nice to update the "development-planning: https://etherpad.opendev.org/p/nova-2025.1-status" to "development-planning: https://etherpad.opendev.org/p/nova-2025.2-status" in the topic if Uggla agrees. i think the rest of the topic looks correct
17:20:25 melwitt the above uploaded patch in the backscroll (https://review.opendev.org/c/openstack/nova/+/949235) looks like one of the garbage AI ones
17:20:27 sean-k-mooney melwitt: i got asked about that issue downstream, found the bug and was like oh melwitt has a fix up
17:20:50 melwitt sean-k-mooney: haha fun
17:21:01 sean-k-mooney melwitt: i have been thating to the reporter a bit
17:21:17 sean-k-mooney not the reportor
17:21:31 sean-k-mooney the sumbitter of https://review.opendev.org/c/openstack/nova/+/949235
17:21:50 sean-k-mooney so they are not really familary with workign in openstack and i think git
17:22:06 sean-k-mooney but ya the commit message is not forllwoing or normal convetion
17:22:25 sean-k-mooney is more like what you do on mailing list where for each revision to expalin what you changed form the last one
17:22:25 melwitt yeah. it's such a tiny thing to fix I thought might as well + backport it. I agree with you that the long term thing will be to stop formatting eph disks
17:22:34 melwitt sean-k-mooney: the commit message claims stuff that isn't in the patch though, like what is that
17:22:46 melwitt huh ok
17:22:57 sean-k-mooney melwitt: so it explainging what has change over time
17:23:06 sean-k-mooney whic is fine in a mailing list btu weired in gerrit
17:23:24 sean-k-mooney also the lost the change id so https://review.opendev.org/c/openstack/nova/+/945204/6
17:23:31 opendevreview Ishan Shanware proposed openstack/nova master: [FEAT]: using quota_details to optimize validate networks [FIX]: added error handling for .get() statements [FIX]: pep8 formatting fixed for changes [FIX]: added additional unit tests changes to mock validate_networks [FIX]: fixes using autopep8 https://review.opendev.org/c/openstack/nova/+/945204
17:23:31 sean-k-mooney is actully the review they shoudl of updated
17:23:49 melwitt I see
17:24:31 sean-k-mooney ishanwar[m]: so you now have two reviews for the same thing because you lost the change id at one point
17:25:34 sean-k-mooney ishanwar[m]: i think you ment to locally squash the two pathces before doing git review
17:25:47 opendevreview Ishan Shanware proposed openstack/nova master: [FEAT]: using quota_details to optimize validate networks [FIX]: added error handling for .get() statements [FIX]: pep8 formatting fixed for changes [FIX]: added additional unit tests changes to mock validate_networks [FIX]: fixes using autopep8 https://review.opendev.org/c/openstack/nova/+/945204
17:27:03 sean-k-mooney ishanwar[m]: ok ^ is better but as i was dissusing with melwitt above you shoudl update the commit message to expalin what the current revion of the patch is changing not how the patch changed over time
17:27:32 opendevreview Ishan Shanware proposed openstack/nova master: [FEAT]: using quota_details to optimize validate networks [FIX]: added error handling for .get() statements [FIX]: pep8 formatting fixed for changes [FIX]: added additional unit tests changes to mock validate_networks [FIX]: fixes using autopep8 https://review.opendev.org/c/openstack/nova/+/945204
17:27:55 sean-k-mooney ishanwar[m]: https://wiki.openstack.org/wiki/GitCommitMessages document the convetions we try to follow
17:27:59 fungi sean-k-mooney: i can update it, though separately i recommend the nova team consider adding some channel operators at https://opendev.org/openstack/project-config/src/branch/master/accessbot/channels.yaml#L239
17:28:18 opendevreview Ishan Shanware proposed openstack/nova master: [FEAT]: using quota_details to optimize validate networks [FIX]: added error handling for .get() statements [FIX]: pep8 formatting fixed for changes [FIX]: added additional unit tests changes to mock validate_networks [FIX]: fixes using autopep8 https://review.opendev.org/c/openstack/nova/+/945204
17:28:49 sean-k-mooney fungi: oh i didnt know we could do that. i assuem we cant just update the topic via a git reivew?
17:29:10 sean-k-mooney fungi: i htink we used to be able to do this in the chanel before we moved to oftc
17:29:19 opendevreview Ishan Shanware proposed openstack/nova master: [FEAT]: using quota_details to optimize validate networks [FIX]: added error handling for .get() statements [FIX]: pep8 formatting fixed for changes [FIX]: added additional unit tests changes to mock validate_networks [FIX]: fixes using autopep8 https://review.opendev.org/c/openstack/nova/+/945204
17:29:26 sean-k-mooney but its been so long i kind of forget the detials
17:30:18 ishanwar[m] sean-k-mooney: i have fixes the pep8 issue kindly have a look whenver you can
17:30:28 fungi sean-k-mooney: you'd add people to an ops list for the channel in that file, then any of them can ask chanserv to set the channel topic like the example in https://docs.opendev.org/opendev/system-config/latest/irc.html#basic-channel-operator-commands
17:31:03 sean-k-mooney fungi: ah cool ill add that to the irc metting adgenda for next week so
17:31:19 sean-k-mooney and we can see what folks think
17:31:26 ishanwar[m] sean-k-mooney: yep changing that
17:31:51 fungi but also if Uggla wants the topic updated in the meantime, i'm happy to do that myself too
17:34:39 opendevreview Ishan Shanware proposed openstack/nova master: [FEAT]: This feat optimises the validate_networks method by reducing networks calls to the database. Here we utilise the quota_details extension to gather the list of uses ports using the neutron extension, this is a smarter query as we don't have to list down all the ports which are in use. Instead we now utilise the quota details extension to get the count of used ports directly. ht
17:39:36 opendevreview Ishan Shanware proposed openstack/nova master: [FEAT]: using quota_details to optimize validate networks https://review.opendev.org/c/openstack/nova/+/945204
17:40:28 opendevreview Ishan Shanware proposed openstack/nova master: [FEAT]: This feat optimises the validate_networks method by reducing networks calls to the database. Here we utilise the quota_details extension to gather the list of uses ports using the neutron extension, this is a smarter query as we don't have to list down all the ports which are in use. Instead we now utilise the quota details extension to get the count of used ports directly. ht
17:41:15 opendevreview Ishan Shanware proposed openstack/nova master: [FEAT]: using quota_details to optimize validate networks https://review.opendev.org/c/openstack/nova/+/945204
17:41:30 ishanwar[m] Sorry for the spam everyone
17:41:56 ishanwar[m] i didn't realize opendev has been sending a notification here all this time
17:42:23 sean-k-mooney ya it does it also send an email to those on the review
17:43:02 sean-k-mooney its not that distruptive but its goit to do those iteration locally and then push a reivew
17:43:09 opendevreview Ishan Shanware proposed openstack/nova master: [FEAT]: This feat optimises the validate_networks method by reducing networks calls to the database. Here we utilise the quota_details extension to gather the list of uses ports using the neutron extension, this is a smarter query as we don't have to list down all the ports which are in use. Instead we now utilise the quota details extension to get the count of used ports directly. ht
17:43:28 sean-k-mooney it avoids the spam and the ci churn but that comes with practice
17:44:31 ishanwar[m] you can have a look, whenever you can
17:44:31 ishanwar[m] I think i have made all the changes i had to right now
17:44:31 ishanwar[m] yeah sorry about that again, will be more careful from now on.
17:57:38 opendevreview melanie witt proposed openstack/nova master: libvirt: Use common naming convention for ephemeral disk labels https://review.opendev.org/c/openstack/nova/+/947541
17:58:29 melwitt sean-k-mooney: added code comment to the test and a release note ^
18:00:13 opendevreview Merged openstack/nova master: wsgi: Don't create, use lock in same line https://review.opendev.org/c/openstack/nova/+/949161
22:52:07 opendevreview sean mooney proposed openstack/nova master: Allow autopep8 to fix more things https://review.opendev.org/c/openstack/nova/+/949292
22:52:07 opendevreview sean mooney proposed openstack/nova master: update pre-commit version pins https://review.opendev.org/c/openstack/nova/+/949291
#openstack-nova - 2025-05-09
00:35:17 opendevreview sean mooney proposed openstack/nova master: reorder and extend pre-commit hooks https://review.opendev.org/c/openstack/nova/+/949299
00:35:18 opendevreview sean mooney proposed openstack/nova master: enable bandit and fix high and medium severity issues https://review.opendev.org/c/openstack/nova/+/949300
06:17:01 opendevreview Ishan Shanware proposed openstack/nova master: [FEAT]: using quota_details to optimize validate networks https://review.opendev.org/c/openstack/nova/+/945204
07:02:37 Uggla fungi, sean-k-mooney tbh I have just noticed the topic is incorrect. fungi it is ok to change it if you can. However there is no hurry it can wait Tuesday and we can ask gmaan too as he seems to be op on this channel.
07:20:55 gibi sean-k-mooney: thank for the review on the pooling. I replied in the review, including your valid comment about the fixtures as well there.
07:32:59 frickler Uggla: ^^ please check

Earlier   Later