Skip to content

Resolve "Migrate SQLAlchemy ORM usage to v2 syntax"

Øyvind Evju requested to merge 2237-migrate-sqlalchemy-orm-usage-to-v2-syntax into dev

Description

Lots of changes here, mostly are about changing session.query to session.execute(select(...) which is SQLAlchemy v2 syntax and a lot cleaner IMO, as it is closer to SQL.

The following changes were done to simplify this job:

## Add ELLAUndesiredUsageWarning

Wrap session.query so that it raises a ELLAUndesiredUsageWarning so that we avoid using session.query.

Add Mypy plugin

Create mypy plugin to warn when session.query is used over session.execute

Also checks when .scalars()/.scalar_one()/.scalar_one_or_none() is used on a select with multiple columns.

Improve typing

Type up lots of functions, especially the ones using Session - this allowed intellisense and mypy to aid in completing the shift. It also ended up satisfying mypy, so this also closes #2238 (closed) .

Make ExtendedQuery redundant

Since we no longer use session.query, we no longer use ExtendedQuery. However, I have not removed the class completely, as there is code there we want to keep.

The function extended_query.ExtendedQuery.temp_table has been copied to extended_query.temp_table, so we can use it as temp_table(session, select(...), name) where session.query(...).temp_table(name) was used before.

Change how sessions are created and closed

Sessions should now be properly cleaned up, and avoid leaving sessions open in "idle in transaction" state. However, this should be tested more on staging

Remove unused functions

Functions that were clearly unused (not covered by tests and not referenced anywhere) has been removed.

Notes to review (code/docs/QA)

I went through this test by test, and fixing ELLAUndesiredUsageWarning, so most of the commits should be fairly easy to follow. However, there is way too much to go through thoroughly, and the only way to really test this is to run it on staging.

After all tests were done, I went through all other usages of session.query and changed this.

These changes open up a lot of other possible improvements, including, but not limited to:

  • Change select.filter to select.where (warn on using select.filter?) to get even closer to SQL syntax.
  • Remove unused sessions from queries
  • Remove ExtendedQuery
  • Run mypy with check_undefined_defs (and other options?)

Related issues

Closes #2237 (closed) Closes #2238 (closed)

Edited by Øyvind Evju

Merge request reports