kvesteri / sqlalchemy-continuum

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

Add support to SQLAlchemy 2.0 #326

Closed josecsotomorales closed 1 year ago

zikphil commented 1 year ago

Any ideas of the scope of work required for this and/or when it will be tackled? :)

zikphil commented 1 year ago

This is still just experimental but so far I had to make those changes:

diff --git a/sqlalchemy_continuum/fetcher.py b/sqlalchemy_continuum/fetcher.py
index a1f2684..7926368 100644
--- a/sqlalchemy_continuum/fetcher.py
+++ b/sqlalchemy_continuum/fetcher.py
@@ -70,11 +70,10 @@ class VersionObjectFetcher(object):
             attrs = alias.c
         query = (
             sa.select(
-                [func(
+                func(
                     getattr(attrs, tx_column_name(obj))
-                )],
-                from_obj=[table]
-            )
+                )
+            ).select_from(table)
             .where(
                 sa.and_(
                     op(
@@ -127,7 +126,7 @@ class VersionObjectFetcher(object):
         alias = sa.orm.aliased(obj.__class__)

         subquery = (
-            sa.select([sa.func.count('1')], from_obj=[alias.__table__])
+            sa.select(sa.func.count('1')).select_from(alias.__table__)
             .where(
                 getattr(alias, tx_column_name(obj))
                 <
@@ -137,7 +136,7 @@ class VersionObjectFetcher(object):
             .label('position')
         )
         query = (
-            sa.select([subquery], from_obj=[obj.__table__])
+            sa.select(subquery).select_from(obj.__table__)
             .where(
                 sa.and_(*eqmap(identity, (obj.__class__, obj)))
             )
diff --git a/sqlalchemy_continuum/manager.py b/sqlalchemy_continuum/manager.py
index 7bb5f73..ff3e139 100644
--- a/sqlalchemy_continuum/manager.py
+++ b/sqlalchemy_continuum/manager.py
@@ -362,7 +362,7 @@ class VersioningManager(object):

         :param session: SQLAlchemy session object
         """
-        if session.transaction.nested:
+        if session.in_nested_transaction():
             return
         conn = self.session_connection_map.pop(session, None)
         if conn is None:
diff --git a/sqlalchemy_continuum/schema.py b/sqlalchemy_continuum/schema.py
index 83728ef..7f51565 100644
--- a/sqlalchemy_continuum/schema.py
+++ b/sqlalchemy_continuum/schema.py
@@ -116,20 +116,19 @@ def get_property_mod_flags_query(
                 getattr(v2.c, tx_column_name).is_(None)
             )).label(column + mod_suffix)
             for column in tracked_columns
-        ],
-        from_obj=v1.outerjoin(
-            v2,
-            sa.and_(
-                getattr(v2.c, end_tx_column_name) ==
-                getattr(v1.c, tx_column_name),
-                *[
-                    getattr(v2.c, pk) == getattr(v1.c, pk)
-                    for pk in primary_keys
-                    if pk != tx_column_name
-                ]
-            )
+        ]
+    ).select_from(v1.outerjoin(
+        v2,
+        sa.and_(
+            getattr(v2.c, end_tx_column_name) ==
+            getattr(v1.c, tx_column_name),
+            *[
+                getattr(v2.c, pk) == getattr(v1.c, pk)
+                for pk in primary_keys
+                if pk != tx_column_name
+            ]
         )
-    ).order_by(getattr(v1.c, tx_column_name))
+    )).order_by(getattr(v1.c, tx_column_name))

 def update_property_mod_flags(
diff --git a/sqlalchemy_continuum/unit_of_work.py b/sqlalchemy_continuum/unit_of_work.py
index 22596b2..d849653 100644
--- a/sqlalchemy_continuum/unit_of_work.py
+++ b/sqlalchemy_continuum/unit_of_work.py
@@ -224,10 +224,9 @@ class UnitOfWork(object):
         )
         if session.connection().engine.dialect.name == 'mysql':
             return sa.select(
-                [sa.text('max_1')],
-                from_obj=[
-                    sa.sql.expression.alias(subquery.subquery() if hasattr(subquery, 'subquery') else subquery, name='subquery')
-                ]
+                sa.text('max_1')
+            ).select_from(
+                sa.sql.expression.alias(subquery.subquery() if hasattr(subquery, 'subquery') else subquery, name='subquery')
             )
         return subquery
Jerommeke4 commented 1 year ago

There is another issue with the polymorphic_on check in model_builder.py which is not able to work with MappedColumn types. The following diff will support this.

--- a/sqlalchemy_continuum/model_builder.py
+++ b/sqlalchemy_continuum/model_builder.py
@@ -2,7 +2,7 @@ from copy import copy
 import six
 import sqlalchemy as sa
 from sqlalchemy.ext.declarative import declared_attr
-from sqlalchemy.orm import column_property
+from sqlalchemy.orm import column_property, MappedColumn
 from sqlalchemy_utils.functions import get_declarative_base

 from .utils import adapt_columns, option
@@ -78,6 +78,8 @@ def copy_mapper_args(model):
             column = model.__mapper_args__['polymorphic_on']
             if isinstance(column, six.string_types):
                 args['polymorphic_on'] = column
+            elif isinstance(column, MappedColumn):
+                args['polymorphic_on'] = column.column.key
             else:
                 args['polymorphic_on'] = column.key
     return args
AlTosterino commented 1 year ago

Bump 💪

marksteward commented 1 year ago

Hi sorry, will look at this shortly

POD666 commented 1 year ago

Bump 🙂

josecsotomorales commented 1 year ago

Bump 🚀

alliefitter commented 1 year ago

If this isn't going to be address any time soon, could y'all at least update setup.py to exclude incompatible versions of dependencies as a stopgap? I just checked out the repo to see if I could get a PR together, but I can't even run the tests because of a problem with flask-sqlalchemy.

plugins/test_flask.py:5: in <module>
    from flask_sqlalchemy import SQLAlchemy, _SessionSignalEvents
E   ImportError: cannot import name '_SessionSignalEvents' from 'flask_sqlalchemy'
alliefitter commented 1 year ago

Got a PR up to address this. Dealing with a few broken tests and some hanging with postgres. I don't have much experience with sqlalchemy, so any help figuring out the problem would be greatly appreciated.

marksteward commented 1 year ago

Hi all, thanks very much for your help. I'm working through the suggested changes and the other issues that are currently causing tests to break but it's taken longer than expected.

In the meantime, I've raised https://github.com/kvesteri/sqlalchemy-continuum/issues/332 in case anyone has opinions on that.

marksteward commented 1 year ago

Just to update here, I'm having trouble with the postgres dialect (particularly the new executemany code, which now appears to be a single query), so I'll have to park this for a few days.

marksteward commented 1 year ago

Right, I've pushed to the sqla-2.0 branch and tests are passing except for the native ones. Is anyone able to have a look through for any issues?

This version only supports SQLAlchemy >= 1.4, but I figure that's reasonable as it's the transitional version and 1.3 is EOL.

Still to do:

I'll also be renaming the master branch to main with this release, just so people are aware.

josecsotomorales commented 1 year ago

Do we have any updates on this?

marksteward commented 1 year ago

I was hoping someone would be able to give it a test with their code, but failing that I'll probably do a release next week.

josecsotomorales commented 1 year ago

@marksteward I'm happy to help in whatever is needed to get this released, just let me know.

josecsotomorales commented 1 year ago

FYI tested the WIP branch in a personal project with SA and PostgreSQL, seems to be working fine.

JPereira-FJB commented 1 year ago

Hi. Do we have any updates? Does SQLAlchemy-Continuum support SQLAlchemy 2.0 in any of the branches?

josecsotomorales commented 1 year ago

I've tested on this branch: https://github.com/kvesteri/sqlalchemy-continuum/tree/sqla-2.0 ... seems to be working fine for PostgreSQL. @marksteward what's needed to release this?

josecsotomorales commented 1 year ago

I'm happy to contribute to whatever we need to get this done.

marksteward commented 1 year ago

Thanks! I'm about to cut a 1.4 release, but if you want to work out any outstanding 2.0 warnings that'd be great. I'm expecting to have to make some patch releases soon after this.

Also, I've just realised I don't have permission to rename master to main. @kvesteri can you do that?

marksteward commented 1 year ago

OK, I've published 1.4. Please give it a go!

josecsotomorales commented 1 year ago

Thanks @marksteward! Will keep testing now with this release