-
Notifications
You must be signed in to change notification settings - Fork 128
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fixes for SQLAlchemy 1.4 #260
Conversation
_decl_class_registry was moved by 450f5c0d. This should be replaced by a public interface once one is added.
Since 5ec5b0a6, SQLAlchemy re-triggers after_configured while we're trying to add version classes. Prevent this handler from being re-entered, but don't make it one-shot.
4ecd352a adds a reference to .entity in AliasedInsp, which previously would accept an object and alias its class.
useexisting was removed in a9b068ae
Hi, what's needed to progress this PR? |
Hey guys, happy to help on adding support to sqlalchemy 1.4 as well, this PR seems like a good starting point. |
gentle ping @kvesteri |
OK, I've had a deeper look at this, and realised I wasn't running the tests properly. After fixing the tests that were being obscured by this, the tests now pass against sqlite: 1.3 (old tests): I needed to pin sqlalchemy-i18n==1.0.3 to get these to work, because 1.1.0 has a breaking change to support sqlalchemy 1.4, which results in errors like this:
I've also added a GitHub action to run the tests. You can see the results here: I'll aim to add tests for different DB engines tomorrow. |
I think the sqlalchemy-i18n issue was temporary, |
OK, that's looking good now: https://github.com/marksteward/sqlalchemy-continuum/actions/runs/1705716581 I'm not sure there's much point testing different python versions across the full matrix, so I'm sticking to 3.8, as it's the default. |
OK, I think these are the minimal changes required to cut a release. It should be backwards-compatible but I'm open to creating a full point release. @kvesteri if I merge this PR, can you publish to pypi or should I sort out a GitHub action for that too? Other stuff involves refreshing dependencies and fixing all the sqla warnings, which should be in a future release. |
@marksteward I can add you as a collaborator in SA-Continuum PyPi. Can you give me your PyPi username? |
@kvesteri marksteward (https://pypi.org/user/marksteward/). Cheers! |
This is an attempt to fix #255. It's WIP but tests now pass basically as they did for 1.3:
1.3:
341 failed, 307 passed, 12 skipped, 650 warnings, 92 errors
1.4:
344 failed, 305 passed, 12 skipped, 5320 warnings, 91 errors
I'd like to work through the huge number of warnings, but I'm happy to leave that to a separate PR if we want this merged.
I've put a summary of issues at #255 (comment) as there's already a conversation there.
Feel free to grab me (ms7821) on IRC to discuss anything.