| hberaud | bnemec: o/ Please can you have a look to this backport https://review.opendev.org/c/openstack/oslo.messaging/+/744262 Thanks by advance. | 07:12 |
|---|---|---|
| hberaud | zzzeek: o/ ironic folks face issue with the latest version of oslo.db (https://review.opendev.org/c/openstack/requirements/+/805611) they think that this could be related to the "remove use of metadata.bind arguments" (https://opendev.org/openstack/oslo.db/commit/22b44ee18b1585bc2943a29ddaa18051cb1344ed). For now this version of oslo.db is not our upper-constraints, so only cross tests | 07:34 |
| hberaud | are impacted. Please can you have a look when you'll have spare time. Here are the logs https://zuul.opendev.org/t/openstack/build/bbaf8fed127d4df2882db50fe2a30a41. Thanks by advance | 07:34 |
| opendevreview | likui proposed openstack/oslo.config master: setup.cfg: Replace dashes with underscores https://review.opendev.org/c/openstack/oslo.config/+/807122 | 11:58 |
| zzzeek | hberaud: yes there is a bug in ironic | 13:37 |
| hberaud | ack | 13:37 |
| zzzeek | hberaud: they will need to convert all implicit execution to explicit | 13:38 |
| zzzeek | if oslo.db is in fact moving forward with these changes | 13:39 |
| zzzeek | hberaud: all of these https://github.com/openstack/ironic/blame/df92f8089eef5925bd827ea2705b40916633fbca/ironic/tests/unit/db/sqlalchemy/test_migrations.py#L225 | 13:39 |
| hberaud | zzzeek: this is related to the metadata.bind or does it have nothing to do with it? | 13:40 |
| hberaud | I see thanks for the link | 13:40 |
| zzzeek | hberaud: it is related yes, it's a SQLAlhcemy API that has been discouraged for some years now and is now deprecated | 13:40 |
| hberaud | got it | 13:40 |
| zzzeek | hberaud: https://docs.sqlalchemy.org/en/14/changelog/migration_20.html#implicit-and-connectionless-execution-bound-metadata-removed | 13:41 |
| zzzeek | hberaud: it's not trivial as you see in oslo.db and i was surprised oslo.db moved ahead as fast as it did | 13:41 |
| hberaud | stephenfin did a great work with these patches | 13:42 |
| hberaud | stephenfin: o/ please can you have a look to https://review.opendev.org/c/openstack/oslo.messaging/+/744262 | 14:46 |
| stephenfin | hberaud: Did we ever figure out why the patch looks differently on stable/train to stable/ussuri, i.e. why can't we use the exception directly rather than adding them to oslo_messaging.exceptions ? | 14:50 |
| stephenfin | compare https://review.opendev.org/c/openstack/oslo.messaging/+/743647/1/oslo_messaging/_drivers/impl_rabbit.py (Ussuri) with https://review.opendev.org/c/openstack/oslo.messaging/+/744262/3/oslo_messaging/_drivers/impl_rabbit.py (Train) | 14:51 |
| hberaud | stephenfin: yes we already discussed this point through the both patches | 14:51 |
| stephenfin | I know we discussed it, but I didn't get a clear answer | 14:51 |
| bnemec | I find that commit message confusing. It says ConnectionForced requires a reboot, but the exception handler says it's recoverable? | 14:51 |
| stephenfin | It's very muddled because of the confusion with the other test failures | 14:52 |
| zzzeek | hberaud: dogpile is released | 14:52 |
| hberaud | zzzeek: thanks | 14:52 |
| hberaud | stephenfin: I'm not sure to see why the import fail with the original version, but I can reproduce it all the time locally | 14:52 |
| stephenfin | Oh yeah, that is weeeeird | 14:54 |
| opendevreview | Merged openstack/oslo.config master: setup.cfg: Replace dashes with underscores https://review.opendev.org/c/openstack/oslo.config/+/807122 | 15:03 |
| hberaud | stephenfin: however I can't reproduce it with the repl... the import seems to works as expected... | 15:09 |
| stephenfin | hberaud: Ah, I've seen this before | 15:10 |
| stephenfin | It's Python 2.7's broken import system | 15:10 |
| hberaud | and the problem was only with py27 | 15:11 |
| stephenfin | We're doing the import in 'oslo_messaging/_drivers/impl_rabbit.py' | 15:11 |
| stephenfin | and there's another file in the same directory, 'oslo_messaging/_drivers/amqp.py' | 15:11 |
| hberaud | ah good catch | 15:11 |
| stephenfin | so 'import amqp' is essentially doing 'import .amqp' or 'from oslo_messaging._drivers import amqp' | 15:11 |
| hberaud | I think you are right | 15:11 |
| stephenfin | python2.7--- | 15:11 |
| hberaud | how to get around that? | 15:12 |
| opendevreview | Stephen Finucane proposed openstack/oslo.messaging stable/train: Catch ConnectionForced Exception https://review.opendev.org/c/openstack/oslo.messaging/+/744262 | 15:17 |
| stephenfin | hberaud: Like that ^ | 15:17 |
| opendevreview | Stephen Finucane proposed openstack/oslo.messaging stable/train: Catch ConnectionForced Exception https://review.opendev.org/c/openstack/oslo.messaging/+/744262 | 15:18 |
| stephenfin | I'm +2 on that now since I think my changes are minimal | 15:19 |
| stephenfin | As a general observation though, we should really try to document every single change in a backport, including why it was necessary. It makes the lives of reviewers so much easier and ensure we're actually backporting the same effective thing as opposed to an entirely new patch | 15:20 |
| stephenfin | They are stable branches, after all :) | 15:20 |
| opendevreview | Merged openstack/oslo.messaging stable/train: Catch ConnectionForced Exception https://review.opendev.org/c/openstack/oslo.messaging/+/744262 | 19:59 |
| *** bnemec is now known as bnemec-pto | 21:45 | |
Generated by irclog2html.py 2.17.2 by Marius Gedminas - find it at https://mg.pov.lt/irclog2html/!