upgrading for SQLAlchemy 2.0+

[crooksey]

Open for discussion, but think this is all that's needed, also backwards compatible.

@ergo any thoughts?

[ergo]

Did you try this? The tests are not passing for me. I will take a look when I'll have the chance.

[crooksey]

I tested this code, you are right that some tests now fail. I have updated the conftest.py to use the 2.0 method of "execute" for SQLAlchemy, there are a few more tests to now individually work on, but I have pushed the changes to the "conftest" file for now.

[ergo]

Sounds great - now the main question is - will the changes still work on 1.x? If not maybe it makes more sense to release 1.x as it is and then release 2.x with alchemy 2 compatibility.

[crooksey]

The actual code changes work on 1.X and 2.X, I haven’t checked the tests (yet). If the 2.0 method for engine execute is not backwards compatible to 1.X I imagine I can resolve this with a try/except code block.With the current code I have I have pushed there is one test failing on 2.X for group resource permissions, this may need your expertise as I have never used this service and don’t fully understand it.— LukeOn 11 Jan 2025, at 01:06, Marcin Lulek ***@***.***> wrote: Sounds great - now the main question is - will the changes still work on 1.x? If not maybe it makes more sense to release 1.x as it is and then release 2.x with alchemy 2 compatibility. —Reply to this email directly, view it on GitHub, or unsubscribe.You are receiving this because you authored the thread.Message ID: ***@***.***>

[crooksey]

So my new changes ARE backwards compatible, so thats all good.

The test error I also get on SQLAlchemy 1.4x, its on the test:

def test_resources_with_group_permission(

All other tests pass fine, the error code for the failed test is:

================================================================================================== FAILURES ==================================================================================================
__________________________________________________________________________ TestUserPermissions.test_resources_with_group_permission __________________________________________________________________________

self = <ziggurat_foundations.tests.test_permissions.TestUserPermissions object at 0x7fd0c51f8910>, db_session = <sqlalchemy.orm.session.Session object at 0x7fd0c4698050>

    def test_resources_with_group_permission(self, db_session):
        created_user = add_user(db_session)
        resource = add_resource(db_session, 1, "test_resource")
        resource2 = add_resource(db_session, 2, "test_resource2")
        # add_resource(db_session, 3, "test_resource3")
        group = add_group(db_session)
        group.users.append(created_user)
        group_permission = GroupResourcePermission(
            perm_name="test_perm", group_id=1, resource_id=resource.resource_id
        )
        group_permission2 = GroupResourcePermission(
            perm_name="foo_perm", group_id=1, resource_id=resource2.resource_id
        )
        resource.group_permissions.append(group_permission)
        resource2.group_permissions.append(group_permission2)
        db_session.flush()
>       resources = UserService.resources_with_perms(
            instance=created_user, perms=["foo_perm"], db_session=db_session
        )

ziggurat_foundations/tests/test_permissions.py:268: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
ziggurat_foundations/models/services/user.py:111: in resources_with_perms
    query = query.outerjoin(
.tox/python/lib/python3.11/site-packages/sqlalchemy/orm/query.py:2488: in outerjoin
    return self.join(target, onclause=onclause, isouter=True, full=full)
<string>:2: in join
    ???
.tox/python/lib/python3.11/site-packages/sqlalchemy/sql/base.py:279: in _generative
    x = fn(self, *args, **kw)
<string>:2: in join
    ???
.tox/python/lib/python3.11/site-packages/sqlalchemy/orm/base.py:306: in generate
    fn(self, *args, **kw)
.tox/python/lib/python3.11/site-packages/sqlalchemy/orm/query.py:2443: in join
    join_target = coercions.expect(
.tox/python/lib/python3.11/site-packages/sqlalchemy/sql/coercions.py:395: in expect
    resolved = impl._literal_coercion(
.tox/python/lib/python3.11/site-packages/sqlalchemy/sql/coercions.py:1233: in _literal_coercion
    self._raise_for_expected(element, argname)
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 

self = <sqlalchemy.sql.coercions.JoinTargetImpl object at 0x7fd0c6233080>
element = (<class 'ziggurat_foundations.tests.conftest.GroupResourcePermission'>, <sqlalchemy.sql.elements.BooleanClauseList object at 0x7fd0c445ead0>), argname = None, resolved = None, advice = None
code = None, err = None, kw = {}, got = "(<class 'ziggurat_foundations.tests.conftest.GroupResourcePermission'>, <sqlalchemy.sql.elements.BooleanClauseList object at 0x7fd0c445ead0>)"
msg = "Join target, typically a FROM expression, or ORM relationship attribute expected, got (<class 'ziggurat_foundations.tests.conftest.GroupResourcePermission'>, <sqlalchemy.sql.elements.BooleanClauseList object at 0x7fd0c445ead0>)."

    def _raise_for_expected(
        self,
        element: Any,
        argname: Optional[str] = None,
        resolved: Optional[Any] = None,
        *,
        advice: Optional[str] = None,
        code: Optional[str] = None,
        err: Optional[Exception] = None,
        **kw: Any,
    ) -> NoReturn:
        if resolved is not None and resolved is not element:
            got = "%r object resolved from %r object" % (resolved, element)
        else:
            got = repr(element)
    
        if argname:
            msg = "%s expected for argument %r; got %s." % (
                self.name,
                argname,
                got,
            )
        else:
            msg = "%s expected, got %s." % (self.name, got)
    
        if advice:
            msg += " " + advice
    
>       raise exc.ArgumentError(msg, code=code) from err
E       sqlalchemy.exc.ArgumentError: Join target, typically a FROM expression, or ORM relationship attribute expected, got (<class 'ziggurat_foundations.tests.conftest.GroupResourcePermission'>, <sqlalchemy.sql.elements.BooleanClauseList object at 0x7fd0c445ead0>).

.tox/python/lib/python3.11/site-packages/sqlalchemy/sql/coercions.py:518: ArgumentError

[crooksey]

@ergo I have had another look, but still can't work out why this is failing, It may be todo with the way "outerjoin" now works in 2.0, but I can't see any issues with breaking changes on the changelog for it.