crate / sqlalchemy-cratedb

SQLAlchemy dialect for CrateDB.
https://cratedb.com/docs/sqlalchemy-cratedb/
Apache License 2.0
3 stars 2 forks source link

SA20: `AttributeError: can't set attribute 'returning'` #110

Closed amotl closed 1 year ago

amotl commented 1 year ago

Observations

CI tripped over the SQLAlchemy 2.0 GA release last night [1]. The same thing can be observed at the recent PR build of crate/crate-python#488 [2].

   File "/home/runner/work/crate-python/crate-python/src/crate/client/sqlalchemy/compat/core14.py", line 95, in visit_update
    crud_params = _get_crud_params(
  File "/home/runner/work/crate-python/crate-python/src/crate/client/sqlalchemy/compat/core14.py", line 192, in _get_crud_params
    compiler.returning = []
AttributeError: can't set attribute

[1] Latest nightly: https://github.com/crate/crate-python/actions/runs/4020995995 [2] PR: https://github.com/crate/crate-python/actions/runs/4023530229/jobs/6914508982#step:4:341

Analysis

Works

pip install "sqlalchemy==2.0.0rc3" --upgrade --pre
./bin/test -t test_nested_object_change_tracking

Croaks

pip install "sqlalchemy==2.0.0" --upgrade
./bin/test -t test_nested_object_change_tracking
[...]
  File "/Users/amo/dev/crate/drivers/crate-python/.venv/lib/python3.10/site-packages/sqlalchemy/sql/compiler.py", line 703, in __init__
    self.string = self.process(self.statement, **compile_kwargs)
  File "/Users/amo/dev/crate/drivers/crate-python/.venv/lib/python3.10/site-packages/sqlalchemy/sql/compiler.py", line 748, in process
    return obj._compiler_dispatch(self, **kwargs)
  File "/Users/amo/dev/crate/drivers/crate-python/.venv/lib/python3.10/site-packages/sqlalchemy/sql/visitors.py", line 143, in _compiler_dispatch
    return meth(self, **kw)  # type: ignore  # noqa: E501
  File "/Users/amo/dev/crate/drivers/crate-python/src/crate/client/sqlalchemy/compat/core14.py", line 95, in visit_update
    crud_params = _get_crud_params(
  File "/Users/amo/dev/crate/drivers/crate-python/src/crate/client/sqlalchemy/compat/core14.py", line 192, in _get_crud_params
    compiler.returning = []
AttributeError: can't set attribute 'returning'
amotl commented 1 year ago

Problem

The reason why this croaks is because the sqlalchemy.compat.core14 adapter gets selected, while running on SA20. The reason for that again is, because distutils' LooseVersion does not cope well and trips the version dispatcher.

https://github.com/crate/crate-python/blob/e8c633faf874694687dab1c3b1028d7fe6e8ccb2/src/crate/client/sqlalchemy/sa_version.py#L22-L28

https://github.com/crate/crate-python/blob/e8c633faf874694687dab1c3b1028d7fe6e8ccb2/src/crate/client/sqlalchemy/dialect.py#L158-L166

Exercise

>>> from distutils.version import LooseVersion as LV
>>> from distutils.version import StrictVersion as SV

>>> SV("2.0.0") >= SV("2.0.0b1")
True
>>> LV("2.0.0") >= LV("2.0.0b1")
False

Root cause

Turtles all the way down, we had to use LooseVersion over StrictVersion with 9f1814318, in order to be able to parse 2.0.0rc1, because StrictVersion would croak on that.

>>> SV("2.0.0rc1")
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/usr/local/Cellar/python@3.10/3.10.8/Frameworks/Python.framework/Versions/3.10/lib/python3.10/distutils/version.py", line 40, in __init__
    self.parse(vstring)
  File "/usr/local/Cellar/python@3.10/3.10.8/Frameworks/Python.framework/Versions/3.10/lib/python3.10/distutils/version.py", line 137, in parse
    raise ValueError("invalid version number '%s'" % vstring)
ValueError: invalid version number '2.0.0rc1'

Solution

Removing that commit will probably make things work again.

amotl commented 1 year ago

Removing that commit (9f181431) will probably make things work again.

It does, see crate/crate-python#488.