opendevreview | Merged openstack/ironic master: Add test timeout to tox config https://review.opendev.org/c/openstack/ironic/+/885372 | 04:15 |
---|---|---|
rpittau | 1 merged, 2 to go | 06:47 |
rpittau | good morning ironic! o/ | 06:47 |
opendevreview | Verification of a change to openstack/ironic master failed: Allow setting migrations timeout value from tox https://review.opendev.org/c/openstack/ironic/+/885837 | 06:54 |
opendevreview | Verification of a change to openstack/ironic master failed: Use tox env variables in coverage tests https://review.opendev.org/c/openstack/ironic/+/885507 | 07:37 |
opendevreview | Verification of a change to openstack/ironic master failed: Allow setting migrations timeout value from tox https://review.opendev.org/c/openstack/ironic/+/885837 | 07:37 |
opendevreview | Verification of a change to openstack/ironic master failed: Use tox env variables in coverage tests https://review.opendev.org/c/openstack/ironic/+/885507 | 07:43 |
opendevreview | Riccardo Pittau proposed openstack/ironic master: Allow setting migrations timeout value from tox https://review.opendev.org/c/openstack/ironic/+/885837 | 08:05 |
rpittau | ^ we need this before the rest | 08:05 |
opendevreview | Riccardo Pittau proposed openstack/ironic master: Use tox env variables in coverage tests https://review.opendev.org/c/openstack/ironic/+/885507 | 08:05 |
opendevreview | Verification of a change to openstack/ironic master failed: Use jammy for base jobs https://review.opendev.org/c/openstack/ironic/+/869052 | 08:48 |
opendevreview | Verification of a change to openstack/ironic master failed: Allow setting migrations timeout value from tox https://review.opendev.org/c/openstack/ironic/+/885837 | 11:12 |
*** Continuity__ is now known as Continuity | 11:24 | |
opendevreview | Iury Gregory Melo Ferreira proposed openstack/ironic master: Add DB API for Firmware and Object https://review.opendev.org/c/openstack/ironic/+/883062 | 11:51 |
iurygregory | good morning Ironic | 11:51 |
iurygregory | rpittau, tks for the review o/ I've updated | 11:51 |
rpittau | iurygregory: no problem! and welcome back :) | 11:57 |
iurygregory | tks! | 11:58 |
iurygregory | I'm planning to sue American Airlines after all the problems I had in this travel XD | 11:58 |
rpittau | lol | 12:10 |
iurygregory | XD | 12:13 |
iurygregory | rpittau, tks for the review | 12:58 |
rpittau | yw :) | 12:59 |
dtantsur | folks, looking for a 2nd +2 on https://review.opendev.org/c/openstack/ironic/+/875944 | 12:59 |
dtantsur | I know it's pretty large, but we need to move on with it (and it will probably conflict with other work, e.g. firwmare) | 13:00 |
iurygregory | dtantsur, i will check today | 13:04 |
dtantsur | thx! | 13:46 |
iurygregory | rpittau, my bad the patch had a pep8 issue, so I'm fixing and including the nits you mentioned in the review o/ | 13:48 |
rpittau | ack no worries | 13:48 |
opendevreview | Iury Gregory Melo Ferreira proposed openstack/ironic master: Add DB API for Firmware and Object https://review.opendev.org/c/openstack/ironic/+/883062 | 13:50 |
iurygregory | dtantsur, I've added a +2, will wait till today to see if TheJulia has more thoughts on the patch before approving right now (but later today I will +w) | 14:02 |
rpittau | looks like the MIGRATIONS_TIMEOUT is not being respected by the migrations tests https://review.opendev.org/c/openstack/ironic/+/885837 or there something else going on that causes the timeout:/ | 15:00 |
iurygregory | .-. | 15:19 |
iurygregory | ouch | 15:19 |
iurygregory | rpittau, at least looking in codesearch for MIGRATIONS_TIMEOUT I only see the variable in test_migrations.py | 15:24 |
rpittau | iurygregory: because we're the only project to use it | 15:24 |
rpittau | it's an override | 15:25 |
iurygregory | os.getenv seems correct to me | 15:25 |
rpittau | yeah, it is, but I think we need a gentle=False entry instead of True | 15:25 |
rpittau | doing some tests now | 15:25 |
iurygregory | ack | 15:25 |
rpittau | looks like that's the way, it will abruptly crash but should give us the list of what's failing | 15:31 |
opendevreview | Riccardo Pittau proposed openstack/ironic master: Allow setting migrations timeout value from tox https://review.opendev.org/c/openstack/ironic/+/885837 | 15:34 |
rpittau | alright, time to start packing, see ya next week! o/ | 15:39 |
iurygregory | gentle=False, makes sense | 15:39 |
iurygregory | enjoy the time off rpittau o/ | 15:39 |
opendevreview | Stephen Finucane proposed openstack/ironic master: Add job to test with SQLAlchemy master (2.x) https://review.opendev.org/c/openstack/ironic/+/886020 | 15:56 |
opendevreview | Jay Faulkner proposed openstack/ironic master: Add job to test with SQLAlchemy master (2.x) https://review.opendev.org/c/openstack/ironic/+/886020 | 16:01 |
opendevreview | Jay Faulkner proposed openstack/ironic master: Fix SQLAlchemy engine connection listener https://review.opendev.org/c/openstack/ironic/+/885797 | 16:04 |
opendevreview | Stephen Finucane proposed openstack/ironic master: Add job to test with SQLAlchemy master (2.x) https://review.opendev.org/c/openstack/ironic/+/886020 | 16:06 |
opendevreview | Stephen Finucane proposed openstack/ironic master: Add job to test with SQLAlchemy master (2.x) https://review.opendev.org/c/openstack/ironic/+/886020 | 16:07 |
stephenfin | JayF: Just rebased the job change on top of the fix | 16:07 |
stephenfin | I didn't know that was already done | 16:07 |
JayF | ah, hopefully you got my pep8 fix on the fix then :) | 16:07 |
JayF | yeah you did good stuff | 16:07 |
stephenfin | Just about did \o/ | 16:08 |
JayF | stephenfin: as the neighborhood sqlalchemy magician, has anyone clued you into our current unit test issues? | 16:08 |
stephenfin | they have not | 16:08 |
JayF | so for about a week or two, we've had intermittant unit test failures | 16:08 |
JayF | things seem to be hanging in the db migration tests | 16:09 |
stephenfin | I saw there were timeouts. I'm going to guess they're related | 16:09 |
stephenfin | Ah | 16:09 |
JayF | https://github.com/openstack/ironic/blob/master/ironic/tests/unit/db/sqlalchemy/test_migrations.py#L1342 was the worst offender | 16:09 |
JayF | and was disabled to allow anything at all to pass | 16:09 |
JayF | but it's happening intermittantly on the gate | 16:09 |
JayF | and afaik, rpittau is the only person to reproduce it locally | 16:09 |
JayF | we're working to improve timeouts around tests to create better errors, but I think there's still something getting hung (probably DB-deadlock-hung, but that's a guess) | 16:10 |
JayF | so if you have any insight or thoughts, would love to hear them | 16:10 |
stephenfin | I'm guessing you don't know when it started? There will have been changes in APIs used but I'm guessing most of that should be syntax. We're still not actually using SQLAlchemy 2.x anywhere | 16:11 |
JayF | we could not get it to behave differently under sqla 1.4 v 2.x | 16:12 |
JayF | I think it's probably in the oslo db fixtures, we have a bad migration, or something like that | 16:12 |
JayF | the failures do not directly map to any patch in Ironic or obvious related library release | 16:12 |
stephenfin | Yeah, I was going to say you're not using the most recent versions of oslo.db (or alembic??) either since those are also capped by sqlalchemy 2.x | 16:13 |
stephenfin | *blocked by | 16:13 |
Nisha_Agarwal | JayF, Regarding bug https://bugs.launchpad.net/proliantutils/+bug/2021995 , could you help me to get the iLO firmware version installed on the system at the minimum? This will help to initiate the discussion with iLO firmware team. | 16:13 |
JayF | Nisha_Agarwal: I asked the folks in the UK to update that with the info in our sync meeting like, 15 minutes ago | 16:14 |
JayF | Nisha_Agarwal: IDK when they will take that action but they're aware | 16:14 |
Nisha_Agarwal | Ok. Thanks | 16:14 |
JayF | https://github.com/openstack/ironic/blob/master/ironic/db/sqlalchemy/alembic/versions/163040c5513f_add_firmware_information.py#L44 why is there a trailing comma there? | 16:41 |
JayF | iurygregory: ^ intentional? | 16:41 |
JayF | yeah it's that way in a few places | 16:46 |
* JayF just looking to see if he can ID anything funky with the migrations | 16:46 | |
iurygregory | seems like you found the answer while I was still at lunch =) | 16:58 |
JayF | well, I would've preferred it be the problem | 16:58 |
JayF | than to be explainable lol | 16:58 |
iurygregory | agree | 16:58 |
iurygregory | it would make our life easier | 16:58 |
JayF | I just... dunno how to approach things anymore tbh | 16:59 |
JayF | starting to run out of things to check | 16:59 |
JayF | not being able to repro it locally is brutal | 16:59 |
iurygregory | yeah =( | 17:00 |
iurygregory | anyone noticed that since yesterday metal3-integration is reporting POST_FAILURE? | 20:04 |
JayF | I saw it, and stuffed it deep down until I figure out the OTHER failures in ci | 20:07 |
JayF | :| | 20:07 |
iurygregory | seems like is trying to use a wrong version of CAPIRELEASE | 20:16 |
iurygregory | but also lib/releases.sh jq command not found is not something good .-. | 20:17 |
JayF | Yeah, it's -nv for a reason. We should ensure it gets fixed but we only control some of the moving parts there | 20:17 |
iurygregory | I'm raising this in the k8s slack | 20:20 |
iurygregory | ok first time I see ironic-cross-sushy with TIMED_OUT (CI, why you are doing this?) | 21:03 |
JayF | I'm reasonably sure we might have a real bug | 21:08 |
JayF | but I'm close to the end of my rope | 21:08 |
JayF | I've read so much code | 21:08 |
JayF | tried to understand it | 21:08 |
JayF | but without a local reproduction I feel a bit stuck | 21:08 |
iurygregory | yeah, same .-. | 21:16 |
iurygregory | JayF, want to +W to see if https://review.opendev.org/c/openstack/ironic/+/885837 help us? | 21:18 |
JayF | done | 21:18 |
opendevreview | Merged openstack/ironic master: Allow setting migrations timeout value from tox https://review.opendev.org/c/openstack/ironic/+/885837 | 21:38 |
iurygregory | \o/ | 21:38 |
opendevreview | Iury Gregory Melo Ferreira proposed openstack/ironic master: Use tox env variables in coverage tests https://review.opendev.org/c/openstack/ironic/+/885507 | 21:42 |
iurygregory | had to rebase because gerrit was showing merge conflict | 21:42 |
JayF | o/ | 21:45 |
iurygregory | going to grab some dinner and go to the gym, bbl | 21:47 |
JayF | have a good one | 21:47 |
iurygregory | ty | 21:48 |
opendevreview | Iury Gregory Melo Ferreira proposed openstack/ironic master: metal3-integration NV https://review.opendev.org/c/openstack/ironic/+/886533 | 22:07 |
JayF | iurygregory: +2A | 22:08 |
JayF | iurygregory: I forgot we had made that voting or I would've pushed that, sorry | 22:08 |
iurygregory | JayF, np o/ | 22:17 |
iurygregory | now time to go to the gym | 22:18 |
Generated by irclog2html.py 2.17.3 by Marius Gedminas - find it at https://mg.pov.lt/irclog2html/!