Skip to content

Draft: Upgrade to sqlalchemy 2 & fix db sessions

Mario Oenning requested to merge moson/aurweb:sqlalchemy2 into master
Bump sqlalchemy to version 2.0.17

WIP

Getting sqlachemy version 2 running as well as fixing session management.

Commit 1 is addressing changes with sqlalchemy v2: Mainly the removal of "auto-commit". The current codebase is heavily relying on that feature. We can work around that by using a wrapper that performs the commit.

Commit 2 implements request based sessions using a middle-ware. Ideally we'd go for the dependency injection approach, but that requires a huge amount of changes in the codebase. Now, almost all routes have database interaction, so it should not be too much of concern to create a new session for each request because we'd need it anyways.

Some of the tests only worked properly since every DB operation was run within the same session. Utilizing separate sessions, we need to refresh (re-query) objects from the DB. With the default isolation_level: REPEATABLE READ however, the DB is effectively NOT re-querying data, it simply returns what has been queried before. Thus db.refresh won't reload from the DB. We can either set the isolation level to READ COMMITTED, making db.refresh working as expected or simply close our transaction (forcing the creation of a new one) with db.commit before we access our db object.

References

Session handling:
https://docs.sqlalchemy.org/en/14/orm/contextual.html#using-thread-local-scope-with-web-applications
https://dev.to/uponthesky/python-post-reviewhow-to-implement-a-transactional-decorator-in-fastapi-sqlalchemy-ein
https://github.com/tiangolo/fastapi/issues/726

Isolation level: https://docs.sqlalchemy.org/en/20/orm/session_state_management.html#refreshing-expiring
https://dev.mysql.com/doc/refman/8.0/en/innodb-transaction-isolation-levels.html
https://mydbops.wordpress.com/2018/06/22/back-to-basics-isolation-levels-in-mysql/

  • Check impact change for isolation_level (READ COMMITTED instead of the default REPEATABLE READ).
    We might want to simply use db.commit instead.
  • More (manual/) "real-world" testing
  • Stress testing (benchmarks; DDoS the thing)
  • Ask @artafinde how fast he can downgrade to a previous release in case of issues 😉

Merge request reports

Loading