* TheJulia calls it a day | 00:00 | |
opendevreview | Vanou Ishii proposed openstack/ironic master: Deal with iRMC virtual media incompatibility https://review.opendev.org/c/openstack/ironic/+/823790 | 05:52 |
---|---|---|
vanou | TheJulia: Thanks for comment. Sorry I'm not sure the point you mentioned. Main motivation of this patch is to deal with incompatibility of iRMC firmware related to virtual floppy disk functionality support. | 05:53 |
vanou | And I modfied patch to deal with pep8 error :) | 05:54 |
rpittau | good morning ironic, happy Friday! o/ | 08:44 |
opendevreview | Riccardo Pittau proposed openstack/bifrost master: Fix jinja ansible lint error https://review.opendev.org/c/openstack/bifrost/+/866137 | 09:00 |
mnasiadka | good morning | 09:00 |
mnasiadka | Anybody willing to add second +2 to https://review.opendev.org/c/openstack/bifrost/+/871647 (kolla job rename)? | 09:01 |
janders | hey rpittau, Happy Friday o/ | 09:26 |
rpittau | hey janders :) | 09:26 |
opendevreview | Riccardo Pittau proposed openstack/bifrost master: Fixes "too many spaces after colon" lint error https://review.opendev.org/c/openstack/bifrost/+/872633 | 09:34 |
opendevreview | Riccardo Pittau proposed openstack/bifrost master: Fixes "too many spaces after colon" lint error https://review.opendev.org/c/openstack/bifrost/+/872633 | 09:36 |
opendevreview | Riccardo Pittau proposed openstack/bifrost master: Finally fix jinja[spacing] https://review.opendev.org/c/openstack/bifrost/+/872634 | 09:42 |
opendevreview | Riccardo Pittau proposed openstack/bifrost master: Finally fix jinja[spacing] https://review.opendev.org/c/openstack/bifrost/+/872634 | 09:43 |
iurygregory | good morning Ironic | 11:22 |
kubajj | Good morning ironic! | 11:27 |
opendevreview | Verification of a change to openstack/bifrost master failed: CI: Rename kolla-ansible-ubuntu-bifrost job https://review.opendev.org/c/openstack/bifrost/+/871647 | 12:40 |
opendevreview | Verification of a change to openstack/bifrost master failed: Fix jinja ansible lint error https://review.opendev.org/c/openstack/bifrost/+/866137 | 13:43 |
TheJulia | good morning! | 14:49 |
rpittau | good morning TheJulia :) | 14:49 |
dtantsur | does anyone have access to a node that underwent inspection? | 14:59 |
dtantsur | can you check if inspection_finished_at is populated? | 14:59 |
TheJulia | ... hmm | 15:13 |
TheJulia | I feel like I'm having deja vu | 15:13 |
kubajj | dtantsur, TheJulia: sorry to ask again, I know it is not that much of a priority for you, but I don't have much else to work on now other than https://review.opendev.org/c/openstack/ironic/+/871394/ Could I get a review? | 15:17 |
dtantsur | reviewed | 15:22 |
kubajj | thanks | 15:24 |
rpittau | bye everyone, have a great weekend! o/ | 15:30 |
TheJulia | dtantsur: I feel like I have seen something suspect there at one point, but I just don't remember anymore. And I don't have a working deployment in front of me... | 15:33 |
TheJulia | well that might not be true | 15:33 |
TheJulia | dtantsur: triggering an inspection in a ci environment which has not refreshed yet | 15:34 |
* TheJulia grins | 15:34 | |
dtantsur | :) | 15:34 |
TheJulia | gah, inspection is not installed | 15:34 |
kubajj | dtantsur: I am not sure I understand this https://review.opendev.org/c/openstack/ironic/+/871394/comments/1dcae2b6_a0aad030 . I thouhg this was the only way how to pass parameters to exceptions. | 15:39 |
dtantsur | kubajj: the __init___? yeah, but your __init__ just pass the parameters further. | 15:39 |
dtantsur | if you remove it, you can just as well call the constructor with obj=.., node=... | 15:40 |
TheJulia | dtantsur: yeah, glancing at the code, I'm curious why you ask about that field :) | 15:43 |
dtantsur | TheJulia: wanting to change some logic in BMO | 15:44 |
dtantsur | while testing, I noticed that I don't have inspection_finished_at on my nodes | 15:44 |
dtantsur | but I don't remember if the inspection was actually finished, wanted to double-check, but destroyed the environment before that.. | 15:44 |
dtantsur | ..and now podman hangs ... | 15:46 |
TheJulia | ugh | 15:48 |
TheJulia | so, looking at the code, we set it in a super weird way | 15:48 |
TheJulia | only on the verb change getting saved | 15:48 |
dtantsur | yeah, I wonder if we ever try to do it in INSPECTWAIT | 15:49 |
TheJulia | we don't | 15:49 |
TheJulia | which is kind of what I'm thinking, is we could be going from inspectwait directly to done | 15:49 |
TheJulia | and not recording the update | 15:50 |
dtantsur | yep | 15:50 |
TheJulia | at least, that is my guess and gut feeling glancing at the code | 15:50 |
dtantsur | but I cannot prove this now because my lab machine is misbehaving.. | 15:50 |
TheJulia | I've not looked at the introspection side, but inspectwait is a thing | 15:50 |
TheJulia | and was added later... | 15:50 |
dtantsur | true | 15:50 |
TheJulia | yup | 15:53 |
TheJulia | that is exactly it | 15:53 |
TheJulia | we only run the task in inspectwait | 15:53 |
dtantsur | \o/ | 15:54 |
TheJulia | which means the conditional in the db never gets triggered | 15:54 |
dtantsur | \o/ \o/ | 15:54 |
TheJulia | so the question is... do we stop hiding the save of the time.... or... not ? | 15:54 |
dtantsur | and nobody has noticed for a few years | 15:54 |
TheJulia | yup | 15:54 |
TheJulia | inspection is largely one and done on initial install | 15:54 |
TheJulia | so it makes sense that it was not directly noticed | 15:55 |
TheJulia | inspector.py and explicitly set the time to the field... or api.py ? | 15:57 |
dtantsur | what do we do with other similar fields? | 15:58 |
dtantsur | I suspect I went down the db path by analogy | 15:58 |
TheJulia | yeah, we do the same in the db for provision | 15:58 |
TheJulia | it is always set though | 15:58 |
kubajj | dtantsur: do you mean just like this https://review.opendev.org/c/openstack/ironic/+/871394/comments/1dcae2b6_a0aad030 ? (I don't want to trigger zuul until I understand 😉 ) | 15:59 |
dtantsur | kubajj: I don't quite understand the question, and the gerrit link is showing all comments | 16:02 |
dtantsur | you can always try locally and run unit tests | 16:03 |
TheJulia | out of curiosity, do unit tests seems lower since tox4 dropped? | 16:03 |
* dtantsur does not know | 16:04 | |
kubajj | dtantsur: I mean, what I have works. I just don't understand what you mean by "in favour of the parent class". I was calling the parent class before | 16:04 |
kubajj | Oh, now I see I posted the wrong link. https://paste.opendev.org/show/bdBtRpTYYrNNLLZRPh59/ | 16:04 |
kubajj | This is what I meant ^ | 16:05 |
dtantsur | kubajj: __init__ like https://paste.opendev.org/show/b48rulG60XepuJ02xnSB/ never makes sense, it does not do anything | 16:05 |
dtantsur | * anything that would not be done without it | 16:05 |
dtantsur | self.obj/self.node are probably not used? | 16:06 |
dtantsur | kubajj: https://paste.opendev.org/show/bSbYHQgH37EiIFUnwebb/ | 16:09 |
dtantsur | Child1 and Child2 are equivalent, __init__ of Child1 does not add anything | 16:10 |
kubajj | I see, so I should take the obj and node form IronicException's kwargs, right? | 16:12 |
kubajj | (That's the parent) | 16:12 |
dtantsur | kubajj: look around this file: most exceptions just rely on IronicException. You only need to change the call site to use keyword arguments (I think you're currently passing them positionally) | 16:16 |
kubajj | dtantsur: so I was just overcomplicating it. | 16:18 |
dtantsur | a bit :) | 16:18 |
TheJulia | blah, I feel like I'm questioning my sanity at the moment | 16:23 |
TheJulia | this is what happens when we lack a test :) | 16:23 |
opendevreview | Jakub Jelinek proposed openstack/ironic master: Erase swift inventory entry on node deletion https://review.opendev.org/c/openstack/ironic/+/871394 | 16:28 |
opendevreview | Julia Kreger proposed openstack/ironic master: fix inspectwait logic https://review.opendev.org/c/openstack/ironic/+/872658 | 16:37 |
TheJulia | so, that feels awful, but I explain in the commit message | 16:38 |
dtantsur | I'll take a look on Monday, need to go now | 16:43 |
TheJulia | o/ | 16:49 |
opendevreview | Merged openstack/bifrost master: CI: Rename kolla-ansible-ubuntu-bifrost job https://review.opendev.org/c/openstack/bifrost/+/871647 | 16:54 |
TheJulia | Now that I have filled mailboxes with meeting invites! | 19:19 |
TheJulia | #success | 19:19 |
opendevstatus | TheJulia: Added success to Success page (https://wiki.openstack.org/wiki/Successes) | 19:19 |
* TheJulia always forgets about the bot | 19:19 | |
TheJulia | dtantsur: "inspection_finished_at": "2023-02-03T17:23:20+00:00", <-- in tempest log \o/ Of course, moving it around the state machine more is going to null out the value, so I guess maybe be cautious with its use in general | 20:01 |
opendevreview | Julia Kreger proposed openstack/ironic-specs master: Add modify steps framework https://review.opendev.org/c/openstack/ironic-specs/+/872349 | 21:36 |
opendevreview | Jay Faulkner proposed openstack/ironic master: API support for CRUD node.shard https://review.opendev.org/c/openstack/ironic/+/866235 | 21:54 |
opendevreview | Jay Faulkner proposed openstack/ironic master: Allow port queries by shard list https://review.opendev.org/c/openstack/ironic/+/872361 | 21:54 |
opendevreview | Jay Faulkner proposed openstack/ironic master: Add support for filtering for sharded nodes https://review.opendev.org/c/openstack/ironic/+/872472 | 21:54 |
JayF | ^^ TheJulia only had a couple issues with these, which are fixed. Can folks please find time to review the chain (there's a DB+Object layer PR that didn't need any revision, and already has 1x+2) | 21:54 |
JayF | I'd really, really like to get this merged and completely off my plate. | 21:54 |
JayF | TheJulia: dtantsur: arne_wiebalck: https://github.com/OpenStackweb/ironic-website/pull/57 | 22:09 |
JayF | this is for the zoom link on the blog; I basically sent the link to the list and am linking to the list from that blogpost | 22:09 |
JayF | hopefully this is enough misdirection to prevent us from getting nuked by nefarious robots | 22:09 |
TheJulia | I think they all have +2's from me now | 22:18 |
Generated by irclog2html.py 2.17.3 by Marius Gedminas - find it at https://mg.pov.lt/irclog2html/!