Update GenericAttributeImpl to work with SqlAlchemy 2.0.22#725
Conversation
https://build.opensuse.org/request/show/1139684 by user dgarcia + anag+factory - Add sqlalchemy-2.0.22.patch to make it compatible with SQLAlchemy>=2.0.22, gh#kvesteri/sqlalchemy-utils#725
|
Reported this issue sqlalchemy-continuum/sqlalchemy-continuum#339 and ended up here, what's needed to merge this PR? |
|
@kurtmckee can you take a look at this? |
|
I'm also interested in this to be merged. Currently I have to pin the sqlalchemy version to 2.0.21 |
|
Test suite appears to be failing for unrelated reasons. @danigm Thanks for contributing this fix! |
https://build.opensuse.org/request/show/1139684 by user dgarcia + anag+factory - Add sqlalchemy-2.0.22.patch to make it compatible with SQLAlchemy>=2.0.22, gh#kvesteri/sqlalchemy-utils#725
https://build.opensuse.org/request/show/1139684 by user dgarcia + anag+factory - Add sqlalchemy-2.0.22.patch to make it compatible with SQLAlchemy>=2.0.22, gh#kvesteri/sqlalchemy-utils#725
|
@kurtmckee @kvesteri happy to be a contributor to this project to get this released |
|
Does this fix retain compatibility with 2.0.21? |
|
I've tested this and I don't think it does the right thing for sqla < 2.0.22, including 1.4.
So to avoid breaking anyone who doesn't immediately update to 2.0.22, I think this code needs to count the number of args, and only add the extra |
|
@marksteward I haven't looked at this yet, this was merged to master tough, I will try to get tests to at least pass with the current master version in this PR first, then come back to this, if you can help with a PR |
This change fixes the issue, but I'm not sure if this should be fixed in SQLAlchemy directly.
I've been debugging and found the problem in this call where there's a missing parameter. Looks like the
AttributeImplclass now requires 4 positional parameters, and the register_attribute mapping is not calling this correctly.Fix #719