| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2024-09-03 | |||
| 14:17:12 | sean-k-mooney | dansmith: i dont think refactoring into the funcitonal test is a reasonable ask so i agree we shoudl just fix the exiting test for now | |
| 14:17:32 | dansmith | sean-k-mooney: right, me either, I was just using that to make the point :) | |
| 14:17:43 | sean-k-mooney | dansmith: ok so i disagree that that shoudl be its point | |
| 14:17:59 | sean-k-mooney | it should be testing the hihgh level interface | |
| 14:18:09 | sean-k-mooney | adn the lower leves shoudl be coverd elsewhere | |
| 14:18:23 | sean-k-mooney | btu for now if your happy with yoru proposed solution then lets proceed with that | |
| 14:18:27 | dansmith | again, I don't disagree with the "should" part.. just don't want to lose the "is currently" part | |
| 14:19:55 | sean-k-mooney | so you want to go back to https://review.opendev.org/c/openstack/nova/+/925208/1/nova/tests/unit/virt/test_virt_drivers.py then and add a check that the excption has qemu-img in it | |
| 14:20:04 | dansmith | if we get a process execution error, and that error mentions qemu-img in it, it's pretty obviously a very targeted case I think | |
| 14:20:09 | sean-k-mooney | just to make sure we only skip if that why | |
| 14:20:12 | dansmith | you could even look for "not found" or something if you want to get super targeted | |
| 14:20:13 | dansmith | right | |
| 14:21:06 | sean-k-mooney | if thats your prfernce then lets do that. i dont like skiping test after we tired to do soemthign but i think this is ok for now | |
| 14:21:29 | sean-k-mooney | when swaping to the oslo version hwoever we shoudl be mocking out the image inpection usage | |
| 14:21:46 | jlejeune | sean-k-mooney: +1 | |
| 14:21:53 | dansmith | again, I'm not sure what this has to do with moving to the oslo version | |
| 14:22:16 | dansmith | sean-k-mooney: you know this is *already* merged right? https://review.opendev.org/c/openstack/nova/+/925025 | |
| 14:22:48 | sean-k-mooney | no i was not aware that merged | |
| 14:24:52 | sean-k-mooney | the reason that relevent is while it might be ok to call | |
| 14:24:57 | sean-k-mooney | nova.image.format_inspector.detect_file_format | |
| 14:25:02 | sean-k-mooney | in a unit test inteded to test the driver interface | |
| 14:25:10 | sean-k-mooney | it feel much less ok to call "oslo_utils.imageutils.format_inspector.detect_file_format" | |
| 14:25:28 | dansmith | but we're not doing either of those here right?? | |
| 14:25:33 | dansmith | s/??/?/ | |
| 14:25:56 | sean-k-mooney | that what causing the qemu-image call as far as im aware | |
| 14:26:08 | dansmith | no, it's libvirt actually creating the snapshot | |
| 14:26:12 | dansmith | qemu-img create -f qcow2 ./tmpm1aqe2cp/25c6cab7c98f4cbba03223754b5d2e74.delta | |
| 14:26:19 | dansmith | this is definitely not from format inspector ^ | |
| 14:26:37 | opendevreview | Takashi Kajinami proposed openstack/nova master: Replace deprecated glanceclient exception https://review.opendev.org/c/openstack/nova/+/914761 | |
| 14:26:50 | sean-k-mooney | oh then this is an unrelatd case wehre we alredy depended on qemu-img? | |
| 14:26:56 | dansmith | this is all code that wasn't being tested at all before because it was being ignored since ImageMeta was missing a key and causing the test to bail super early for libvirt | |
| 14:27:00 | sean-k-mooney | i tought this was intoduced by the cve fix | |
| 14:27:21 | sean-k-mooney | oh right you changed it to not return NotImplemented | |
| 14:27:23 | dansmith | sean-k-mooney: yes, I fixed a case where we were not getting the coverage we expected and now we're actually running code we weren't before, that's why I don't want to go back to not covering that | |
| 14:27:25 | dansmith | right | |
| 14:27:27 | jlejeune | by the removal of AMI support actually | |
| 14:27:33 | sean-k-mooney | so its related but not directly because you fixed the missing test coverage | |
| 14:27:34 | dansmith | exactly, the AMI thing, unrelated to anything else | |
| 14:27:35 | sean-k-mooney | sorry | |
| 14:27:38 | opendevreview | Takashi Kajinami proposed openstack/nova master: Replace deprecated glanceclient exception https://review.opendev.org/c/openstack/nova/+/914761 | |
| 14:28:16 | sean-k-mooney | dansmith: with that context then yes im much more ok with your suggeed approch | |
| 14:28:51 | dansmith | cool | |
| 14:28:56 | dansmith | jlejeune: sorry for the runaround :) | |
| 14:29:01 | jlejeune | no prob :) | |
| 14:29:05 | jlejeune | let's do that ! | |
| 15:12:53 | opendevreview | Julien LE JEUNE proposed openstack/nova master: Skip snapshot test when missing qemu-img https://review.opendev.org/c/openstack/nova/+/925208 | |
| 15:14:08 | jlejeune | dansmith: sean-k-mooney: ^ | |
| 15:14:48 | jlejeune | something like that ? | |
| 15:18:03 | dansmith | sure, but that will fail pep8 | |
| 15:18:37 | jlejeune | my bad | |
| 15:18:42 | dansmith | but looks okay to me otherwise | |
| 15:26:07 | opendevreview | Julien LE JEUNE proposed openstack/nova master: Skip snapshot test when missing qemu-img https://review.opendev.org/c/openstack/nova/+/925208 | |
| 15:33:14 | dansmith | sean-k-mooney: I think you commented about using backslash continuations recently on another patch, not sure why you didn't on this one, but I also can't find out guideline about that | |
| 15:33:55 | dansmith | I know we have them in some places and I'm not sure I care about our previous hard and fast rule about it, other than just wanting to be consistent | |
| 15:33:55 | sean-k-mooney | ya i personally hate them but we are not as concistent about not doing it as i would like | |
| 15:34:15 | sean-k-mooney | and im presonally geting very tired of style related review comments | |
| 15:34:21 | dansmith | ah right here, second bullet: https://docs.openstack.org/hacking/latest/user/hacking.html | |
| 15:34:48 | sean-k-mooney | yep its documetned but not enforced by tools | |
| 15:34:51 | dansmith | jlejeune: ^ | |
| 15:34:52 | dansmith | right | |
| 16:02:13 | opendevreview | Julien LE JEUNE proposed openstack/nova master: Skip snapshot test when missing qemu-img https://review.opendev.org/c/openstack/nova/+/925208 | |
| 16:05:50 | erlon | Are we having the nova meeting today? | |
| 16:06:01 | gibi | erlon: no, it is cancelled | |
| 16:06:10 | gibi | bauzas is off today | |
| 16:06:17 | erlon | ah, ok, missed the announcement | |
| 16:06:51 | sean-k-mooney | sylvain is at the opensatck asia summit event in korea currently | |
| 16:07:19 | sean-k-mooney | erlon: did you have a pressing topic ot bring up? | |
| 16:07:25 | gibi | yeah that is the more precise term instead of being off :) | |
| 16:07:36 | erlon | I was to bring the shared group to attention again | |
| 16:07:55 | erlon | we have addressed the last reviews suggestions | |
| 16:07:58 | sean-k-mooney | shared security groups or somethign else? | |
| 16:08:15 | erlon | shared security groups | |
| 16:08:20 | erlon | https://review.opendev.org/c/openstack/nova/+/811521 | |
| 16:08:30 | erlon | from this series: https://review.opendev.org/q/topic:sg-shared-filter | |
| 16:08:34 | sean-k-mooney | ah well for that i think we will liikely jsut reapprove that for next cycle | |
| 16:08:59 | sean-k-mooney | so once RC 1 is creeated in 2 weeks master will reopen for new work and it can proceed | |
| 16:10:28 | erlon | ok, sounds good | |
| 16:10:53 | erlon | is there a way to flag it like that so the team can approve it next? | |
| 16:10:58 | erlon | I | |
| 16:12:04 | sean-k-mooney | put it in the team adjenta for the next meeting and you can link to this irc converstiaon if you like | |
| 16:12:37 | sean-k-mooney | so add it to open disucsion here https://wiki.openstack.org/wiki/Meetings/Nova#Weekly_Nova_team_meeting | |
| 16:13:54 | sean-k-mooney | erlon: skiming the updated version it looks ok but i will need to review it properly later. it looks like you have simplifed it a little since the last version which is nice to see | |
| 16:14:21 | haleyb | sean-k-mooney: i tried to address all the comments | |
| 16:14:51 | sean-k-mooney | haleyb: sorry you are the most recent uploader :) yes i think you have | |
| 16:15:22 | haleyb | i just wanted to move that along, np | |
| 16:15:22 | sean-k-mooney | i basically am getting a headache right now which is why im holding of reviewing it proeprly | |
| 16:15:38 | haleyb | hope you feel better | |
| 16:16:13 | sean-k-mooney | i think its just a lack of drinking water so im goign to go remiday that | |
| 17:22:14 | opendevreview | Merged openstack/nova stable/2024.1: libvirt: call get_capabilities() with all CPUs online https://review.opendev.org/c/openstack/nova/+/927556 | |
| 17:41:26 | opendevreview | Merged openstack/nova stable/2024.1: Fix port group network metadata generation https://review.opendev.org/c/openstack/nova/+/923689 | |
| 17:41:35 | opendevreview | Merged openstack/nova master: Skip snapshot test when missing qemu-img https://review.opendev.org/c/openstack/nova/+/925208 | |
| 19:07:12 | gmann | sean-k-mooney: if you are around https://review.opendev.org/c/openstack/placement/+/926935 | |
| 19:27:21 | sean-k-mooney | +2w | |
| 19:41:04 | gmann | thanks | |
| 20:54:12 | opendevreview | Brian Haley proposed openstack/nova master: Support creating servers with RBAC SGs https://review.opendev.org/c/openstack/nova/+/811521 | |
| 21:34:53 | opendevreview | Merged openstack/placement master: Bump oslo.policy version to enable new RBAC by default https://review.opendev.org/c/openstack/placement/+/926935 | |
| 22:07:57 | opendevreview | Merged openstack/nova master: Remove default override for RBAC config options https://review.opendev.org/c/openstack/nova/+/925519 | |
| #openstack-nova - 2024-09-04 | |||
| 00:55:38 | opendevreview | Merged openstack/nova master: [ironic] Followup fixes around flavor ids https://review.opendev.org/c/openstack/nova/+/927764 | |
| 02:27:29 | melwitt | sean-k-mooney: re: your review comment about being a freeze period.. we are frozen for features but I didn't think we are frozen on merging bug fixes. at least that isn't how it has worked in the past | |
| 02:28:22 | melwitt | if something has changed, I missed it | |