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

Continue migration to SQLAlchemy 2.0 syntax #6057

Closed
2 tasks
bari12 opened this issue Jan 27, 2023 · 5 comments
Closed
2 tasks

Continue migration to SQLAlchemy 2.0 syntax #6057

bari12 opened this issue Jan 27, 2023 · 5 comments
Assignees
Milestone

Comments

@bari12
Copy link
Member

bari12 commented Jan 27, 2023

Description

This is an epic ticket to track the migration of SQLAlchemy 2.0 ORM syntax.

Motivation

https://docs.sqlalchemy.org/en/20/changelog/migration_20.html

Change

  • Create an overview of which Rucio modules are correctly migrated and which ones still need changes (and possible how many)
  • Plan how/who to implement the changes
@rcarpa
Copy link
Contributor

rcarpa commented Feb 1, 2023

Maybe just upgrade to sqlalchemy2 and fix whatever breaks ? Some deprecated constructs like the "query" api are only deprecated on paper and will be probably supported at least till sqlalchemy3 :D we don't "need" to migrate all queries to the new syntax, but the fixes from sqlalchemy2 and type checking support would be welcomed asap

@rcarpa
Copy link
Contributor

rcarpa commented Feb 1, 2023

I just submitted a PR which updates many queries to a format which supports both sqlalchemy1.4 and 2.0: #6060 .

@yuyiguo
Copy link
Contributor

yuyiguo commented Feb 25, 2023

Rucio modules already in SQLAlchemy 2.0 syntax:

  1. lib/rucio/db
  2. lib/rucio/core (partialiy)
  3. lib/rucio/daemons/bb8

Rucio modules need to migrate to SQLAlchemy 2.0 syntax:
lib/rucio/tests
lib/rucio/daemons
lib/rucio/api

These were roughly estimated based the " import sqlalchemy" . We will need details before the assignment

@bari12
Copy link
Member Author

bari12 commented Mar 9, 2023

Hi Yuyi, a bit more details would be good, since this is still quite large categories. Is there some kind of simple test we can just run to see which code is not 2.0 ready?

@yuyiguo
Copy link
Contributor

yuyiguo commented Mar 10, 2023

Hi @bari12
Based on the document , there are seven steps to migrate to 2.0. Let's go through these.

  1. Migration to 2.0 Step One - Python 3 only (Python 3.7 minimum for 2.0 compatibility)
    done
  2. Migration to 2.0 Step Two - Turn on RemovedIn20Warnings
    I tried and did not find a way to do so because we are running in a server or daemon.
  3. Migration to 2.0 Step Three - Resolve all RemovedIn20Warnings
    We cannot do it because we cannot run step 2.
  4. Migration to 2.0 Step Four - Use the future flag on Engine
    We did it in 5872.
  5. Migration to 2.0 Step Five - Use the future flag on Session
    We did it as in step 4.
  6. Migration to 2.0 Step Six - Add allow_unmapped to explicitly typed ORM models
    Not sure how this applies to us.
  7. Migration to 2.0 Step Seven - Test against a SQLAlchemy 2.0 Release
    That is what we need to do.
    As you can see there are no patterns for the migration. Each individual query has to check its own syntax because we missed steps 2 and 3. I searched for the changes suggested by the official doc , as well as made in #6060 and #5872. I found there was something similar in the codes.
    I put the list at the end. We can do a quick fix for all these. Then I suggest that we build rucio with sqlAlchemy 2.0 and run it to replace step two. That will give us things to be fixed.
    Here is the list:
    query = s.query(models.AlembicVersion.version_num)
    to query = s.execute(select(models.AlembicVersion)).scalars().all() and so on.

core/.py
tests/
.py
core/did_meta_plugins/.py
daemons/bb8/
.py
daemons/c3po/collectors/free_space.py
daemons/storage/consistency/actions.py

result = (
    session.query(User)
    .join(User.addresses)
    .distinct()
    .order_by(Address.email_address)
    .all()
)

to

stmt = (
    select(User, Address.email_address)
    .join(User.addresses)
    .distinct()
    .order_by(Address.email_address)
)

result = session.execute(stmt).columns(User).all()

core/config.py
core/rule.py

The Session will no longer support “autocommit” mode,

db/sqla/session.py

@bari12 bari12 assigned erlingstaff and unassigned yuyiguo Jan 12, 2024
erlingstaff added a commit to erlingstaff/rucio that referenced this issue Jan 25, 2024
erlingstaff added a commit to erlingstaff/rucio that referenced this issue Jan 31, 2024
erlingstaff added a commit to erlingstaff/rucio that referenced this issue Feb 1, 2024
rdimaio added a commit to rdimaio/rucio that referenced this issue Feb 19, 2024
rdimaio added a commit to rdimaio/rucio that referenced this issue Feb 20, 2024
rdimaio added a commit to rdimaio/rucio that referenced this issue Feb 27, 2024
rdimaio added a commit to rdimaio/rucio that referenced this issue Mar 5, 2024
bari12 pushed a commit that referenced this issue Mar 7, 2024
* Rules: remove unnecessary arguments in __find_surplus_locks_and_remove_them

* Rules: add type hints; #6454

* Rules: update query to SQLAlchemy 2.0 syntax; #6057

* Rules: Add RuleDict typed dict; #6454
rdimaio added a commit to rdimaio/rucio that referenced this issue Mar 7, 2024
rdimaio added a commit to rdimaio/rucio that referenced this issue Mar 7, 2024
rdimaio added a commit to rdimaio/rucio that referenced this issue Mar 7, 2024
erlingstaff added a commit to erlingstaff/rucio that referenced this issue Mar 11, 2024
erlingstaff added a commit to erlingstaff/rucio that referenced this issue Mar 11, 2024
erlingstaff added a commit to erlingstaff/rucio that referenced this issue Mar 11, 2024
erlingstaff added a commit to erlingstaff/rucio that referenced this issue Mar 11, 2024
erlingstaff added a commit to erlingstaff/rucio that referenced this issue Mar 11, 2024
erlingstaff added a commit to erlingstaff/rucio that referenced this issue Mar 14, 2024
@bari12 bari12 added this to the 34.0.0 milestone Mar 14, 2024
@bari12 bari12 closed this as completed Mar 14, 2024
voetberg pushed a commit to voetberg/rucio that referenced this issue Mar 21, 2024
* Rules: remove unnecessary arguments in __find_surplus_locks_and_remove_them

* Rules: add type hints; rucio#6454

* Rules: update query to SQLAlchemy 2.0 syntax; rucio#6057

* Rules: Add RuleDict typed dict; rucio#6454
voetberg pushed a commit to voetberg/rucio that referenced this issue Mar 21, 2024
voetberg pushed a commit to voetberg/rucio that referenced this issue Apr 15, 2024
* Rules: remove unnecessary arguments in __find_surplus_locks_and_remove_them

* Rules: add type hints; rucio#6454

* Rules: update query to SQLAlchemy 2.0 syntax; rucio#6057

* Rules: Add RuleDict typed dict; rucio#6454
voetberg pushed a commit to voetberg/rucio that referenced this issue Apr 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

No branches or pull requests

4 participants