db: add sqlalchemy 2.0 support

[tobias-urdin]

This adds support for SQLAlchemy 2.0 but does not solve the removal of autocommit which will need to be handled in the future.

[stephenfin]

Only spotted one potential issue. The rest looks good though we'll surely spot other issues when we resolve the autocommit thing and use SQLAlchemy 2.x (and oslo.db 13.x).

Out of curiosity, would it be worth enabling warnings in our test like we've done for most OpenStack projects?

[tobias-urdin]

Only spotted one potential issue. The rest looks good though we'll surely spot other issues when we resolve the autocommit thing and use SQLAlchemy 2.x (and oslo.db 13.x).

Out of curiosity, would it be worth enabling warnings in our test like we've done for most OpenStack projects?

Thanks, yeah the autocommit part will really suck when it hits there is no real control of the transactions so more cleanup required there, the testing right now actually uses SQAlchemy 2.x and newer oslo.db already because we have unpinned dependencies that is not restricted by any upper-constraints.

I did look into enabling warnings with a warnings fixtures to the tests, used it while doing this work but didn't get any noticeable warnings except for the autocommit IIRC.

I opted for unpinning SQLAlchemy pinning and instead add a separate SQLAlchemy 1.4 job to make sure we don't break >=1.4<2 users.

[stephenfin]

Only spotted one potential issue. The rest looks good though we'll surely spot other issues when we resolve the autocommit thing and use SQLAlchemy 2.x (and oslo.db 13.x).
Out of curiosity, would it be worth enabling warnings in our test like we've done for most OpenStack projects?

Thanks, yeah the autocommit part will really suck when it hits there is no real control of the transactions so more cleanup required there, the testing right now actually uses SQAlchemy 2.x and newer oslo.db already because we have unpinned dependencies that is not restricted by any upper-constraints.

Oh really? I wonder how it's working though since autocommit isn't available in SQLAlchemy 2.x (at least outside of driver-level implementations). Weird.

I did look into enabling warnings with a warnings fixtures to the tests, used it while doing this work but didn't get any noticeable warnings except for the autocommit IIRC.

I opted for unpinning SQLAlchemy pinning and instead add a separate SQLAlchemy 1.4 job to make sure we don't break >=1.4<2 users.

Sounds good.

[tobias-urdin]

Only spotted one potential issue. The rest looks good though we'll surely spot other issues when we resolve the autocommit thing and use SQLAlchemy 2.x (and oslo.db 13.x).
Out of curiosity, would it be worth enabling warnings in our test like we've done for most OpenStack projects?

Thanks, yeah the autocommit part will really suck when it hits there is no real control of the transactions so more cleanup required there, the testing right now actually uses SQAlchemy 2.x and newer oslo.db already because we have unpinned dependencies that is not restricted by any upper-constraints.

Oh really? I wonder how it's working though since autocommit isn't available in SQLAlchemy 2.x (at least outside of driver-level implementations). Weird.

I wish I had an answer for that but really I'm just glad it even works, I'm hoping nothing major is overlooked and that it actually works as well, it's still unreleased code so atleast not a catastrophe if this were to break someones CI.

I did look into enabling warnings with a warnings fixtures to the tests, used it while doing this work but didn't get any noticeable warnings except for the autocommit IIRC.
I opted for unpinning SQLAlchemy pinning and instead add a separate SQLAlchemy 1.4 job to make sure we don't break >=1.4<2 users.

Sounds good.

Do you have any other feedback for this or do you approve?

[stephenfin]

Nope, no other feedback. This looks good to me as-is now.