SQLAlchemy 1.4 compatibility

[Glandos]

Hi,
I'm one of the maintainer of https://github.com/spiral-project/ihatemoney/ and we use this great project as a dependency.

We recently discovered that SQLAlchemy 1.4 was released, and it broke our test case. Before providing more details on it, have you ever experienced it?

[killthekitten]

@Glandos could you actually provide more details? I looked briefly at ihatemoney's travis logs, and it doesn't seem related to SQLAlchemy-Continuum per se

[Glandos]

I think our Travis configuration is broken… We didn't take the time to update it since policy change.

Anyway, here is the tox output

py39 installed: alembic==1.5.8,aniso8601==9.0.1,appdirs==1.4.4,attrs==20.3.0,Babel==2.9.0,black==20.8b1,bleach==3.3.0,blinker==1.4,cachetools==4.2.1,certifi==2020.12.5,cffi==1.14.5,chardet==4.0.0,click==7.1.2,colorama==0.4.4,cryptography==3.4.7,debts==0.5,distlib==0.3.1,dnspython==2.1.0,docutils==0.17,email-validator==1.1.2,filelock==3.0.12,flake8==3.9.0,Flask==1.1.2,Flask-Babel==1.0.0,Flask-Cors==3.0.10,Flask-Mail==0.9.1,Flask-Migrate==2.7.0,Flask-RESTful==0.3.8,Flask-Script==2.0.6,Flask-SQLAlchemy==2.5.1,Flask-Testing==0.8.1,Flask-WTF==0.14.3,greenlet==1.0.0,idna==2.10,ihatemoney @ file:///home/adrien/Dev/ihatemoney/.tox/.tmp/package/1/ihatemoney-5.dev0.zip,importlib-metadata==3.10.0,iniconfig==1.1.1,isort==5.8.0,itsdangerous==1.1.0,jeepney==0.6.0,Jinja2==2.11.3,keyring==23.0.1,Mako==1.1.4,MarkupSafe==1.1.1,mccabe==0.6.1,mypy-extensions==0.4.3,packaging==20.9,pathspec==0.8.1,pkginfo==1.7.0,pluggy==0.13.1,py==1.10.0,pycodestyle==2.7.0,pycparser==2.20,pyflakes==2.3.1,Pygments==2.8.1,pyparsing==2.4.7,pytest==6.2.3,python-dateutil==2.8.1,python-editor==1.0.4,pytz==2021.1,readme-renderer==29.0,regex==2021.4.4,requests==2.25.1,requests-toolbelt==0.9.1,rfc3986==1.4.0,SecretStorage==3.3.1,six==1.15.0,SQLAlchemy==1.4.5,SQLAlchemy-Continuum==1.3.11,SQLAlchemy-Utils==0.36.8,toml==0.10.2,tox==3.23.0,tqdm==4.59.0,twine==3.4.1,typed-ast==1.4.2,typing-extensions==3.7.4.3,urllib3==1.26.4,virtualenv==20.4.3,webencodings==0.5.1,Werkzeug==1.0.1,WTForms==2.2.1,zest.releaser==6.22.1,zipp==3.4.1
py39 run-test-pre: PYTHONHASHSEED='3878917591'
py39 run-test: commands[0] | python --version
Python 3.9.2

[…]

ImportError while importing test module '/home/user/Dev/ihatemoney/.tox/py39/lib/python3.9/site-packages/ihatemoney/tests/api_test.py'.
Hint: make sure your test modules/packages have valid Python names.
Traceback:
/usr/lib/python3.9/importlib/__init__.py:127: in import_module
    return _bootstrap._gcd_import(name[level:], package, level)
/home/user/Dev/ihatemoney/.tox/py39/lib/python3.9/site-packages/ihatemoney/tests/api_test.py:7: in <module>
    from ihatemoney.tests.common.ihatemoney_testcase import IhatemoneyTestCase
/home/user/Dev/ihatemoney/.tox/py39/lib/python3.9/site-packages/ihatemoney/tests/common/ihatemoney_testcase.py:4: in <module>
    from ihatemoney import models
/home/user/Dev/ihatemoney/.tox/py39/lib/python3.9/site-packages/ihatemoney/models.py:16: in <module>
    from sqlalchemy_continuum import make_versioned, version_class
/home/user/Dev/ihatemoney/.tox/py39/lib/python3.9/site-packages/sqlalchemy_continuum/__init__.py:3: in <module>
    from .manager import VersioningManager
/home/user/Dev/ihatemoney/.tox/py39/lib/python3.9/site-packages/sqlalchemy_continuum/manager.py:6: in <module>
    from sqlalchemy_utils import get_column_key
/home/user/Dev/ihatemoney/.tox/py39/lib/python3.9/site-packages/sqlalchemy_utils/__init__.py:1: in <module>
    from .aggregates import aggregated  # noqa
/home/user/Dev/ihatemoney/.tox/py39/lib/python3.9/site-packages/sqlalchemy_utils/aggregates.py:372: in <module>
    from .functions.orm import get_column_key
/home/user/Dev/ihatemoney/.tox/py39/lib/python3.9/site-packages/sqlalchemy_utils/functions/__init__.py:1: in <module>
    from .database import (  # noqa
/home/user/Dev/ihatemoney/.tox/py39/lib/python3.9/site-packages/sqlalchemy_utils/functions/database.py:11: in <module>
    from .orm import quote
/home/user/Dev/ihatemoney/.tox/py39/lib/python3.9/site-packages/sqlalchemy_utils/functions/orm.py:14: in <module>
    from sqlalchemy.orm.query import _ColumnEntity
E   ImportError: cannot import name '_ColumnEntity' from 'sqlalchemy.orm.query' (/home/user/Dev/ihatemoney/.tox/py39/lib/python3.9/site-packages/sqlalchemy/orm/query.py)

[Glandos]

In fact, it seems to be caused by kvesteri/sqlalchemy-utils#474 that should be fixed by merging kvesteri/sqlalchemy-utils#482 or kvesteri/sqlalchemy-utils#506 (albeit the last one is a giant PR).

So… in my opinion, I should let this issue opened until something is done in SQLAlchemy-Utils and the dependency is bumped in continuum. But otherwise, feel free to close it ;)

[killthekitten]

@Glandos unfortunately, I'm not the maintainer, and the repo is a bit stale by now. I would suggest you to fork it if it's blocking you from migration to 1.4

[delyanr]

The above error seems to have been fixed with the recent update of sqlalchemy-utils. Thanks @kvesteri! However, I'm getting another one:

  File "C:\Miniconda3\envs\s39sql\lib\site-packages\sqlalchemy\orm\mapper.py", line 3354, in configure_mappers
    _configure_registries(_all_registries(), cascade=True)
  File "C:\Miniconda3\envs\s39sql\lib\site-packages\sqlalchemy\orm\mapper.py", line 3387, in _configure_registries
    Mapper.dispatch._for_class(Mapper).after_configured()
  File "C:\Miniconda3\envs\s39sql\lib\site-packages\sqlalchemy\event\attr.py", line 256, in __call__
    fn(*args, **kw)
  File "C:\Miniconda3\envs\s39sql\lib\site-packages\sqlalchemy\orm\events.py", line 743, in wrap
    fn(*arg, **kw)
  File "C:\Miniconda3\envs\s39sql\lib\site-packages\sqlalchemy_continuum\builder.py", line 163, in configure_versioned_classes
    self.build_transaction_class()
  File "C:\Miniconda3\envs\s39sql\lib\site-packages\sqlalchemy_continuum\builder.py", line 141, in build_transaction_class
    self.manager.create_transaction_model()
  File "C:\Miniconda3\envs\s39sql\lib\site-packages\sqlalchemy_continuum\manager.py", line 166, in create_transaction_model
    self.transaction_cls = self.transaction_cls(self)
  File "C:\Miniconda3\envs\s39sql\lib\site-packages\sqlalchemy_continuum\factory.py", line 9, in __call__
    registry = manager.declarative_base._decl_class_registry
AttributeError: type object 'Base' has no attribute '_decl_class_registry'

Any possibility to update the package to fix the above? Thanks so much

[muffmolch]

We also use this great site package and would be happy if the compatibility to sql alchemy v1.4 will be fixed!

[gnubyte]

Hey folks,
I'll be looking at this as a new maintainer.

Of course, if anyone makes a fix I'm willing to look at their pull request/merge as well & test it.

Thanks!

[delyanr]

@gnubyte Apologies, I'm swamped at the moment to open a PR. However, if helpful, please see the following links:

Formal discussion on SQLAclhemy github
Fix implemented by the team at flask-resty

[anthraxx]

any chance this can be resolved by any maintainer? We are currently hit by this when maintaining the distro packages leading to broken software 😿

[muffmolch]

Same here. Any chance to get this package working with SQKAlchemy 1.4?

ModuleNotFoundError: No module named 'sqlalchemy.ext.declarative.api'

[dmartin-isp]

Bump! Us too! Pinning to SQLAlchemy 1.3.11 for now.

[delyanr]

@gnubyte Do you plan to look into this issue still? Thanks!

[muffmolch]

Does this help somehow for getting rid of the compatibility issue?

https://stackoverflow.com/a/10775143

[gnubyte]

I believe I am seeing activity via email and other threads regarding 1.4 compatibility patches. On a professional level, I produced a separate repo to bridge the immediate need I had for this library by creating similar functionality with field-level git-backed repos through a new library called gitmixin.

I do however have some time now with my business needs separately met, love open source, and would still be happy to get this patched.

If you are a business or a project that relies on this, I am looking to have this patched by September 17th, pending no pushback from consumer base or fellow contributors.

Thank you!

[anthraxx]

@gnubyte did you have any luck spending time on this? its a major blocker 🐈

[rosensama]

Seems like it was a major change and other projects have having similar issues.

https://docs.sqlalchemy.org/en/14/changelog/migration_14.html#declarative-is-now-integrated-into-the-orm-with-new-features
https://stackoverflow.com/a/66666783/1913816

[marksteward]

@gnubyte I've got some time to look at this now. Can you let me know if you have any shareable work-in-progress, or it's better for me to start from scratch. Cheers!

[muffmolch]

@gnubyte I've got some time to look at this now. Can you let me know if you have any shareable work-in-progress, or it's better for me to start from scratch. Cheers!

@marksteward Do you already have any findings or results?

[meksor]

Hey guys, i have a fork that works for me so far:
https://github.com/meksor/sqlalchemy-continuum

Seems _decl_class_registry is gone and now accessible like this:

class ModelFactory(object):
    model_name = None

    def __call__(self, manager):
        """
        Create model class but only if it doesn't already exist
        in declarative model registry.
        """
        for mapper in manager.declarative_base.registry.mappers:
            clsname = mapper.class_.__name__
            if self.model_name == clsname:
                return mapper.class_
        else:
            return self.create_class(manager)

I got another error in builder.py which I hacked around like this:

    def enable_active_history(self, version_classes):
        """
        Assign all versioned attributes to use active history.
        """
        for cls in version_classes:
            for prop in sa.inspect(cls).iterate_properties:
                impl = getattr(cls, prop.key).impl
                if impl is not None:
                    impl.active_history = True
                    

Disclaimer: I have no idea what impl is and what the issue is when its None, so the above code might be complete insanity.

I would really like to use this package for a new version of https://github.com/iiasa/ixmp for transaction-based versioning.
But: The package doesn't work for batch inserts with insert(table) and until the compatibility issue is fixed it's not really useable at all.

cheers

[marksteward]

OK, I've had a proper look at the changes and various proposed fixes. I've opened a PR at #260 as a starting point.

• I've matched sqlalchemy-utils and used registry._class_registry as it's a minimal change. We should move to the fix implemented by implement decent inspection API for registry -> mapped classes sqlalchemy/sqlalchemy#6080 once that's complete. This was caused by sqlalchemy/sqlalchemy@450f5c0.

• The impl issue is a side-effect of sqlalchemy/sqlalchemy@5ec5b0a. sqla now triggers after_configured re-entrantly when continuum adds classes. This currently means continuum tries to add duplicate relationships, which shows up as warnings like:

  /app/sqlalchemy_continuum/model_builder.py:275: SAWarning: This declarative base already contains a class with the same class name and module name as sqlalchemy_continuum.model_builder.UserVersion, and will be replaced in the string-lookup table.

  It also means impl on relationship attributes is accessed prematurely. I think the access of impl is actually a bug (we should iterate over column_attrs rather than iterate_properties). However, configure_versioned_classes should be protected from re-entry.

• AliasedInsp no longer quietly accepts objects in place of a class, since sqlalchemy/sqlalchemy@4ecd352. This would cause the following error:

  /usr/local/lib/python3.9/site-packages/sqlalchemy/orm/util.py:658: AttributeError: 'InstanceState' object has no attribute 'entity'

• Various deprecated features were removed in sqlalchemy/sqlalchemy@a9b068a. I've fixed the test that relies on useexisting, but mapper.order_by has a dedicated test that I've left in place for now.

If you have any questions, I'm ms7821 in #sqlalchemy on Libera, so feel free to drop in.

[josecsotomorales]

Hey folks, I'm also happy to contribute on adding support to sqlalchemy 1.4, I see an open PR that is a very good start from @marksteward, can we merge it?

[KeironO]

Fantastic news! Can't wait for the fix!

[marksteward]

I've merged in the PR. Is there anyone available to test the main branch before I put it on pypi?

[marksteward]

OK, I've now released 1.3.12. Please let me know if you see any problems.