| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2023-09-01 | |||
| 14:50:22 | auniyal | sean-k-mooney, should we catch one more i.e only Exception in manager.py | |
| 14:50:40 | auniyal | after EndpointNotFound | |
| 14:50:56 | bauzas | sean-k-mooney: looking | |
| 14:51:19 | sean-k-mooney | auniyal: no we shoud avoid catching Exception in general | |
| 14:51:27 | bauzas | yeah | |
| 14:51:35 | sean-k-mooney | i think we even have a hacking check to prevent you doing that without a #noqa line | |
| 14:51:47 | sean-k-mooney | s/line/comment/ | |
| 14:52:04 | bauzas | no AFAIR | |
| 14:52:10 | bauzas | but meh, anyway | |
| 14:52:25 | auniyal | sean-k-mooney ack | |
| 14:52:55 | bauzas | sean-k-mooneydoesn't see in your patch where you're moving the attachment_get_call(), just see it added at first | |
| 14:53:11 | bauzas | but I assume you're moving the call, not copying it | |
| 14:53:13 | auniyal | actually I was saying it because its here https://github.com/openstack/nova/blob/master/nova/compute/manager.py#L3184 | |
| 14:53:26 | sean-k-mooney | im moving it im still editing but i just wanted to confirm that was the general direction | |
| 14:53:38 | bauzas | yeah, that's the direction we agreed | |
| 14:53:38 | auniyal | but yes, it should be avoided | |
| 14:53:47 | bauzas | make it the first call in the method and bail out early | |
| 14:54:03 | bauzas | this way, non-cinder envs would meh this method | |
| 14:54:07 | sean-k-mooney | yep and im adding a second early our for | |
| 14:54:12 | sean-k-mooney | len(bdms_to_delete) == 0: | |
| 14:54:23 | sean-k-mooney | after the first loop | |
| 14:54:30 | bauzas | well, we're looping, right ? | |
| 14:55:10 | bauzas | ah | |
| 14:55:25 | bauzas | then after the loop, my bad, you just told it :) | |
| 14:55:59 | sean-k-mooney | https://termbin.com/a7sc | |
| 14:56:13 | sean-k-mooney | ok im goign to start fixign the test and ill push somthing up in a bit | |
| 14:57:04 | dansmith | sean-k-mooney: I'd be okay with that other than the dangling paren | |
| 14:57:24 | sean-k-mooney | for the excpet | |
| 14:57:28 | dansmith | and the unnecessary wrapping of the except list | |
| 14:57:31 | dansmith | that could be two lines but is four | |
| 14:58:05 | sean-k-mooney | well our convention is if you need to wrap you move all args | |
| 14:58:29 | dansmith | no it's not | |
| 14:59:01 | sean-k-mooney | it depend on if you want to dedent the args | |
| 14:59:02 | dansmith | that's black convention that has been sneaking into nova over years | |
| 14:59:10 | sean-k-mooney | either we dont denent them or we do and then move all | |
| 14:59:43 | dansmith | first example I found in the same file: https://review.opendev.org/c/openstack/nova/+/893489/1/nova/compute/manager.py#3881 | |
| 14:59:54 | dansmith | there's another right above it | |
| 15:00:11 | sean-k-mooney | yep i know we have been converting but fine i have swap back to that | |
| 15:00:38 | dansmith | "we" have not been converting, AFAIK.. "some people" have been converting :) | |
| 15:00:39 | sean-k-mooney | i dont really mind doing that in this case as it not hevvily indented | |
| 15:01:08 | sean-k-mooney | i really hate when we have really indeneted arges across like 10 lins | |
| 15:01:22 | sean-k-mooney | when it can be 4 if we dedent and wrap properly | |
| 15:01:43 | dansmith | well, regardless, this is not that case | |
| 15:01:47 | sean-k-mooney | we are both trying not to have lots of wasted virtical space so im ok to do it the other way here | |
| 15:04:51 | dansmith | yep | |
| 15:41:58 | sean-k-mooney | ok i think i have things wokring now | |
| 15:42:28 | sean-k-mooney | i cant do the second early out | |
| 15:42:58 | opendevreview | sean mooney proposed openstack/nova master: only attempt to clean up dangling bdm if cinder is installed https://review.opendev.org/c/openstack/nova/+/893502 | |
| 15:47:08 | sean-k-mooney | ok i need to run now so dansmith bauzas can ye take this from here. | |
| 15:47:20 | bauzas | ack | |
| 16:20:40 | bauzas | dansmith: can you please validate https://review.opendev.org/c/openstack/nova/+/893502 ? | |
| 16:23:48 | dansmith | bauzas: +W.. we might want to get a depends-on running against it in parallel to make sure it really solves the problem | |
| 16:23:59 | dansmith | was that a devstack base job that hit the original problem? | |
| 16:24:10 | bauzas | dansmith: yeah, some neutron ones | |
| 16:24:18 | bauzas | https://bugs.launchpad.net/tempest/+bug/2033752 | |
| 16:24:48 | dansmith | neutron ones or devstack ones? | |
| 16:25:11 | dansmith | ykarel: still around? | |
| 16:25:12 | bauzas | sorry https://zuul.openstack.org/job/neutron-ovn-base | |
| 16:25:47 | ykarel | dansmith, yes kind off | |
| 16:25:48 | bauzas | https://zuul.openstack.org/job/tempest-integrated-networking doesn't skip cinder, but his child above skips it | |
| 16:26:03 | dansmith | ykarel: I'll submit a DNM neutron patch to test | |
| 16:26:44 | dansmith | ykarel: https://review.opendev.org/c/openstack/neutron/+/893538 | |
| 16:26:51 | ykarel | dansmith, ack | |
| 16:27:33 | bauzas | I'll bail out now | |
| 16:27:39 | dansmith | bauzas: ack | |
| 16:27:41 | bauzas | long day for a long week | |
| 16:28:00 | dansmith | bauzas: puh lease man.. back for one week after gone for what, three? | |
| 16:28:11 | dansmith | I'm going to really enjoy that ONE day off I have next week ;) | |
| 16:28:15 | bauzas | 3 and a half :) | |
| 16:28:21 | dansmith | pssh | |
| 16:28:32 | bauzas | yeah, but going back on the ff week, lovin' <3 | |
| 16:29:04 | dansmith | bauzas: go on, get out of here.. I don't want to over stress your system :) | |
| 16:29:14 | bauzas | dansmith: anyway, enjoy your own timeoff :) | |
| 16:29:20 | dansmith | heh thanks | |
| 16:46:56 | auniyal | dansmith - https://bf80d1d7066e6c071f87-1fbe10cd6d23547f6092d75bc2b558ab.ssl.cf2.rackcdn.com/893502/6/check/nova-ovs-hybrid-plug/4042471/job-output.txt - "DBNonExistentDatabase" | |
| 16:47:37 | auniyal | and https://zuul.opendev.org/t/openstack/build/40424718c3524bb78ca22cb9d5293b6c/log/controller/logs/screen-n-cpu.txt#14946 - this one is oslo_service.periodic_task | |
| 16:48:21 | dansmith | auniyal: what about it? | |
| 16:48:49 | auniyal | nova-ovs-hybrid-plug job failed with POST fail | |
| 16:49:07 | auniyal | because devstack@c-bak.service | |
| 16:49:28 | dansmith | oh, sean changed the zuul yaml | |
| 16:49:31 | dansmith | I didn't even look at that | |
| 16:49:34 | auniyal | couldn't start | |
| 16:49:49 | dansmith | yeah we should just do that in a different patch | |
| 16:50:03 | dansmith | auniyal: can you update sean's patch to just not do the .zuul part and I'll +2+W it? | |
| 16:50:23 | dansmith | move the .zuul part to a second patch on top and we can go from there separate from fixing the gate | |
| 16:50:34 | auniyal | ack | |
| 16:50:46 | dansmith | auniyal: can you do it quick? if not I will, but it would be better if you do and I +W | |
| 16:51:00 | auniyal | yes I am doing | |
| 16:51:02 | dansmith | cool | |
| 16:51:32 | dansmith | ++ glad you were watching those jobs live, I didn't even pay attention to the .zuul changer | |
| 16:54:09 | dansmith | something else is re-enabling c-bak, which is why that happened: | |
| 16:54:11 | dansmith | 2023-09-01 16:19:08.457805 | compute1 | +++ enable_service c-bak | |
| 16:56:26 | opendevreview | Amit Uniyal proposed openstack/nova master: only attempt to clean up dangling bdm if cinder is installed https://review.opendev.org/c/openstack/nova/+/893502 | |
| 16:56:27 | opendevreview | Amit Uniyal proposed openstack/nova master: Moved zuul job update https://review.opendev.org/c/openstack/nova/+/893540 | |
| 16:57:26 | dansmith | gmann: can you tell us why this is not working? https://review.opendev.org/c/openstack/nova/+/893540/1/.zuul.yaml | |
| 16:57:45 | dansmith | gmann: c-bak ends up re-enabled but fails to start because the rest of cinder is disabled | |
| 16:58:24 | gmann | dansmith: checking | |
| 16:59:51 | dansmith | auniyal: ah, you didn't remove the comment from the commit message about modifying the job, can you fix that real quick? | |
| 17:00:01 | auniyal | yes | |
| 17:01:27 | sean-k-mooney | without the job config tweak we dont actuly knwo that this is fixed | |