googleapis / python-spanner-sqlalchemy

Apache License 2.0
38 stars 28 forks source link

alembic incompatibility with sqlalchemy < 1.3.11 due to computed columns #269

Closed elainearbaugh closed 1 year ago

elainearbaugh commented 1 year ago

Environment details

Steps to reproduce

  1. install any sqlalchemy version under 1.3.11
  2. run any alembic migration

Easy way to try this using the existing tests--in the migration_test function in noxfile.py, add

session.run("pip", "install", "sqlalchemy>=1.1.13,<=1.3.10", "--force-reinstall")

at the beginning after the imports to force install the sqlalchemy versions with the incompatibilities. Then run the tests using nox -s migration_test. In alembic upgrade head, and the following error should occur:

<removed>
  File "google/cloud/sqlalchemy_spanner/sqlalchemy_spanner.py", line 352, in get_column_specification
    if column.computed is not None:
  File "nox/migration_test/lib/python3.8/site-packages/sqlalchemy/sql/elements.py", line 690, in __getattr__
    key)
AttributeError: Neither 'Column' object nor 'Comparator' object has an attribute 'computed'

This is because the sqlalchemy_spanner alembic code here refers to the computed attribute of Column, which was introduced in sqlalchemy 1.3.11 (release notes, code ref). So, for sqlalchemy versions < 1.3.11, this will throw the AttributeError above since Columns don't have the computed attribute.

A simple but maybe hacky fix would be to just check if the attribute exists before accessing it:

New version

if hasattr(column, "computed") and column.computed is not None:
    colspec += " " + self.process(column.computed)

Old version

if column.computed is not None:
    colspec += " " + self.process(column.computed)

but definitely open to suggestions for how to fix this!

elainearbaugh commented 1 year ago

Created a PR with the fix I mentioned here: https://github.com/googleapis/python-spanner-sqlalchemy/pull/270