kvesteri / sqlalchemy-continuum

Versioning extension for SQLAlchemy.
BSD 3-Clause "New" or "Revised" License
575 stars 127 forks source link

Fixes for SQLAlchemy 1.4 #260

Closed marksteward closed 2 years ago

marksteward commented 2 years ago

This is an attempt to fix https://github.com/kvesteri/sqlalchemy-continuum/issues/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 https://github.com/kvesteri/sqlalchemy-continuum/issues/255#issuecomment-1003629992 as there's already a conversation there.

Feel free to grab me (ms7821) on IRC to discuss anything.

marksteward commented 2 years ago

cc @kvesteri @gnubyte

marksteward commented 2 years ago

Hi, what's needed to progress this PR?

josecsotomorales commented 2 years ago

Hey guys, happy to help on adding support to sqlalchemy 1.4 as well, this PR seems like a good starting point.

anthraxx commented 2 years ago

gentle ping @kvesteri

marksteward commented 2 years ago

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): 655 passed, 97 skipped, 665 warnings 1.3 (new tests): 655 passed, 97 skipped, 657 warnings 1.4 (new tests): 653 passed, 99 skipped, 4909 warnings

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:

AttributeError: 'Comparator' object has no attribute '_copy' AttributeError: Neither 'Column' object nor 'Comparator' object has an attribute '_copy'

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.

marksteward commented 2 years ago

I think the sqlalchemy-i18n issue was temporary, .copy is now showing as deprecated in 1.4 (and using the internal _copy isn't the right fix anyway). If so, I'll pin 1.0.3 here and we can stick with that for a bit.

marksteward commented 2 years ago

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.

marksteward commented 2 years ago

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.

kvesteri commented 2 years ago

@marksteward I can add you as a collaborator in SA-Continuum PyPi. Can you give me your PyPi username?

marksteward commented 2 years ago

@kvesteri marksteward (https://pypi.org/user/marksteward/). Cheers!