Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support to SQLAlchemy 2.0 #326

Closed
josecsotomorales opened this issue Jan 31, 2023 · 22 comments
Closed

Add support to SQLAlchemy 2.0 #326

josecsotomorales opened this issue Jan 31, 2023 · 22 comments

Comments

@josecsotomorales
Copy link
Contributor

No description provided.

@zikphil
Copy link

zikphil commented Mar 9, 2023

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

@zikphil
Copy link

zikphil commented Mar 9, 2023

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
Copy link

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
Copy link

Bump 💪

@marksteward
Copy link
Collaborator

Hi sorry, will look at this shortly

@POD666
Copy link

POD666 commented Apr 5, 2023

Bump 🙂

@josecsotomorales
Copy link
Contributor Author

Bump 🚀

@alliefitter
Copy link

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
Copy link

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
Copy link
Collaborator

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 #332 in case anyone has opinions on that.

@marksteward
Copy link
Collaborator

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.

anthraxx added a commit to archlinux/arch-security-tracker that referenced this issue May 14, 2023
@marksteward
Copy link
Collaborator

marksteward commented Jun 7, 2023

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
Copy link
Contributor Author

Do we have any updates on this?

@marksteward
Copy link
Collaborator

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
Copy link
Contributor Author

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

@josecsotomorales
Copy link
Contributor Author

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

@JPereira-FJB
Copy link

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

@josecsotomorales
Copy link
Contributor Author

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
Copy link
Contributor Author

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

@marksteward
Copy link
Collaborator

marksteward commented Jul 12, 2023

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
Copy link
Collaborator

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

@josecsotomorales
Copy link
Contributor Author

Thanks @marksteward! Will keep testing now with this release

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

8 participants