Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2024-11-20
03:17:27 opendevreview Takashi Kajinami proposed openstack/placement master: Replace deprecated FormatChecker.cls_checks https://review.opendev.org/c/openstack/placement/+/935734
03:29:39 opendevreview melanie witt proposed openstack/nova master: Add [quota]unified_limits_resource_(strategy|list) https://review.opendev.org/c/openstack/nova/+/924025
03:31:30 melwitt dansmith, sean-k-mooney: I also updated the config options patch to reflect the latest PS of the spec ^
04:10:31 opendevreview Takashi Kajinami proposed openstack/nova master: Replace deprecated FormatChecker.cls_checks https://review.opendev.org/c/openstack/nova/+/935733
04:33:15 opendevreview melanie witt proposed openstack/nova master: Add [quota]unified_limits_resource_(strategy|list) https://review.opendev.org/c/openstack/nova/+/924025
05:46:14 opendevreview Rajesh Tailor proposed openstack/nova master: Add support for showing finish_time https://review.opendev.org/c/openstack/nova/+/928933
06:01:59 opendevreview Merged openstack/nova master: Remove workaround for eventlet < 0.27.0 https://review.opendev.org/c/openstack/nova/+/930726
09:54:52 frickler bauzas: (or other cores) seems we still do need sean-k-mooney's fix for os-vif https://review.opendev.org/c/openstack/os-vif/+/935653 in order to be able to get https://review.opendev.org/c/openstack/os-vif/+/932436 in in order to unblock oslo.utils in reqs, please have a look
10:08:58 opendevreview Takashi Kajinami proposed openstack/os-vif master: DNM: Testing 932436 https://review.opendev.org/c/openstack/os-vif/+/935755
10:24:07 opendevreview Merged openstack/nova-specs master: Re-propose config option for behavior of unset unified limits https://review.opendev.org/c/openstack/nova-specs/+/934391
12:12:42 gibi elodilles: Hi! Can I get review on the next set of patches in the backort series of https://review.opendev.org/q/topic:%22bug/2085975%22
12:15:03 gibi frickler: I fast approved the os-vif change to get things unblocked
12:23:26 elodilles gibi: looking
12:29:36 elodilles +2'd them
12:50:57 gibi elodilles: thanks a lot
12:55:11 elodilles np
14:20:05 tkajinam sean-k-mooney, hi o/ Could you check my reply in https://review.opendev.org/c/openstack/nova/+/931582 when you have time ?
14:21:55 sean-k-mooney do you plan to add that funcitonaltiy in oslo in the future?
14:22:27 sean-k-mooney im ok with having it in nova in the interim but i think it woudl be better to have it in oslo eventurally
14:22:52 tkajinam yeah I can move it to oslo.utils
14:23:11 sean-k-mooney ok i wont block on that then we can just do that when it makes sense to do so
14:23:46 tkajinam I also replaced netifaces used in oslo.utils by psutil so the proposed implementation is consistent with the ones we maintain in oslo.utils
14:24:11 tkajinam so there is not strict blocker for future migration
14:24:15 sean-k-mooney im not sure if nova is already usign psutils but i was hoping to aovid a direct dep if we were not
14:24:30 tkajinam it does.
14:24:51 tkajinam https://github.com/openstack/nova/blob/master/requirements.txt#L51
14:25:00 sean-k-mooney https://github.com/openstack/nova/blob/e8a544f0e1134a4b260720cc634ad1997f936d6d/nova/virt/libvirt/volume/quobyte.py#L22
14:25:04 sean-k-mooney for that
14:25:15 sean-k-mooney https://github.com/openstack/nova/blob/e8a544f0e1134a4b260720cc634ad1997f936d6d/nova/virt/libvirt/volume/quobyte.py#L55-L62
14:25:26 tkajinam it's for quobyte, yeah
14:25:48 sean-k-mooney that code looks suspect as we really shoudl not be trying to look at systemd and if its running
14:25:55 sean-k-mooney but ok
14:26:03 sean-k-mooney since its already a dep it does not impact packaging
14:26:16 tkajinam we deprecated quobyte support so that code may go away next cycle
14:26:27 sean-k-mooney well it moving form optional to requried but its also requried because fo oslo so i guess its fine
14:26:41 tkajinam yes
14:27:39 sean-k-mooney ok so i have only one other comment, there is no new testcoverage for get_machine_ips
14:28:01 sean-k-mooney you removed https://review.opendev.org/c/openstack/nova/+/931582/6/nova/tests/unit/compute/test_utils.py
14:28:29 sean-k-mooney the error case but didnt provide any repalcement coverage. and with the poison that impoiles this is not called in other tests or is mocked
14:28:51 sean-k-mooney so i dont think we have any coverage of that code.
14:30:00 sean-k-mooney can you add a small testcase that mocks psutil.net_if_addrs() and jsut cover the body of the for
14:30:32 sean-k-mooney i.e. i.e. the removal of the ipv6 adresses? im not sure how hard that would be
14:31:04 sean-k-mooney sory its not remivng the ipv6 adresses
14:31:21 sean-k-mooney btu it is stripign the content after %
14:32:14 tkajinam that removes scope from ipv6 address
14:32:17 sean-k-mooney i guess we didnt have coverate for that before...
14:32:24 tkajinam let me update the change to include unit tests. That's a very good point
14:32:26 sean-k-mooney so its technially not a regression
14:32:51 sean-k-mooney im kind of +1.5
14:33:18 sean-k-mooney i dont have a sense of how worried i shoudl be about a regression in that code
14:33:49 sean-k-mooney you didnt actully chagne andy of the logic that is processing the adresses
14:34:24 sean-k-mooney you just removed the try excpet
14:35:03 sean-k-mooney presumably because `psutil.net_if_addrs().items()` alwasy returns a dict even if its an empty one
14:35:45 sean-k-mooney or well an iterator over a mapping/dict view
14:36:42 tkajinam yeah
14:36:43 sean-k-mooney tkajinam: im kind of feelign that the missing unit test is a prexisiting bug and i should not ask you to fix it while your are fixing the netifaces issue
14:37:45 tkajinam sean-k-mooney, that's also ok. I can add some test coverage in an independent follow-up
14:38:29 tkajinam yeah it reminds me that I only removed the test case because the exception may no be raised in a new implementation and there are no actual test coverage for normal cases
14:40:00 sean-k-mooney i left a coment to that effect and a +2
14:40:38 sean-k-mooney if you have time to add coverage in a followup then that great if not its ok
14:41:10 sean-k-mooney ill leave it to others to disagree on the reivew if they feel differently
14:42:31 tkajinam sean-k-mooney, thank you !
14:44:15 sean-k-mooney frickler: looks liek even with the exteneed timeout it can still sometiems fail
14:45:00 sean-k-mooney i think for now it would be best to skip that test when using the native driver
14:45:32 sean-k-mooney it always passes for the ovs-vsctl one and we can revisti this once the current set of release are done
14:45:45 sean-k-mooney *requirements bumps
14:49:01 sean-k-mooney unfortunetly we do not capture the ovs logs in this job but i suspect there might be an error of some kidn that we can see
14:49:35 sean-k-mooney s/can see/can't see/
15:10:05 ildikov bauzas: sean-k-mooney: Hi, I'm pinging you about the Bridging the Gap effort that we last discussed at the recent PTG.
15:10:08 ildikov We talked about some simple improvements for Nova, like documenting your process to set priorities for the work that's happening during release cycles.
15:11:48 ildikov I was looking into the Nova docs a little bit, and was doing a Google search on this topic as well to see what a newcomer might find when looking for information about how to work with the Nova team.
15:12:07 ildikov I found a page in the Nova docs that talks about specs, blueprints, etc: https://docs.openstack.org/nova/latest/contributor/blueprints.html
15:12:42 ildikov It looks like a good place to add some additional information about processes that the team is using throughout release cycles to organize your work.
15:12:46 ildikov What do you think?
15:13:56 sean-k-mooney that is one place but not the only one we have
15:14:19 sean-k-mooney this is the starting point https://docs.openstack.org/nova/latest/contributor/contributing.html
15:14:38 sean-k-mooney the doc you linked to is fpr mew featire [;ammomg
15:15:11 sean-k-mooney here you will see the more detailed docs https://docs.openstack.org/nova/latest/contributor/index.html
15:16:41 sean-k-mooney ildikov: we maintian an entire subseciton of our docs "the contibutor guide/docs"
15:16:52 sean-k-mooney so we can add more contet to this as required
15:17:12 sean-k-mooney a high level overview of our process is here https://docs.openstack.org/nova/latest/contributor/process.html
15:17:51 sean-k-mooney ildikov: https://docs.openstack.org/nova/latest/contributor/process.html#why-do-we-have-priorities is where we discuss priorities but we have nto enforced them for some time
15:18:21 sean-k-mooney this was the section we added most recently https://docs.openstack.org/nova/latest/contributor/process.html#what-the-review-priority-label-in-gerrit-are-use-for
15:19:03 sean-k-mooney to document how to sue review priority label but bauzas proposed a diffent way to do it via etherpads last cycle which has more or less replaced it
15:19:28 sean-k-mooney we also have old documetion of the runwasy system we used before that https://docs.openstack.org/nova/latest/contributor/process.html#runways
15:19:59 sean-k-mooney ildikov: so i can see why it might be confusitn since we still have to docs for the previous ways of working
15:20:34 sean-k-mooney ildikov: this is somthing that is continuiously evloving as we try thigns for a release or two and then try somethign new
15:21:29 sean-k-mooney so nova has a lot of docs on this topic already but most have not been updated for a few releases
15:22:21 sean-k-mooney ildikov: that partly why i tough the fedback toward nova was rather unfare in the ptg discussions
15:23:25 sean-k-mooney ildikov: it seamed to ignroe all of the existing doc we had and the efforts we had made to make it transparent
16:06:56 dansmith melwitt: okay I see, it's being echoed once because of the manage_output=$() and once for the [[ $manage_output =~ SUCCESS ]]
16:08:37 dansmith melwitt: so it might be better to do this:
16:08:49 dansmith nova_manage blah | tee /tmp/output
16:08:57 dansmith if grep SUCCESS /tmp/output;
16:09:24 dansmith so we get one copy
16:10:05 melwitt the job output is so hard to parse for me :( ok, I'll try that
16:10:06 dansmith also I just realized you're not checking the return code and just looking for the SUCCESS, but a script will be using the former, so we should probably as well yeah?
16:10:59 dansmith yeah I want the job output to be easier to read for sure and not stuffing large multi-line outputs into vars will help I think
16:12:27 melwitt yeah that would work, originally I didn't have it returning non zero for the warning case and I didn't think to change it
16:12:29 melwitt I'll do that also

Earlier   Later