Review encapsulation of SQLAlchemy and Psycopg2 DBALs
Working on issue #4251 revealed, that the SQLAlchemy and/or Psycopg2 DBALs may not be optimally used within the project. For example cursors or sessions my be abused in a way that goes against best practices, which can cause troubles like those in #4251, because sessions/transaction can get stuck.
It would be wise to review current implementation.
Updated by Radko Krkoš 8 months ago
This issue stems from #4251, where a workaround for a specific transaction handling bug was found and recommended. Based on the videocall discussion a more streamlined approach is required, possibly by managing the transaction lifetime by limiting the life of a
Following the official documentation , the upcoming version of
SQLAlchemy (v1.4), currently in beta, includes a
Python's context manager based workflow with
Session as a transaction manager and
Engine as some form of a connection manager.
Based on , the best approach would be to keep a reference to
Engine, instantiate a
Session from it using a context manager and perform the data-focused actions inside that context. This is perhaps better documented by a code excerpt from :
# create session and add objects with Session(engine) as session, session.begin(): session.add(some_object) session.add(some_other_object) # inner context calls session.commit(), if there were no exceptions # outer context calls session.close()
The current version 1.3 does not support the context manager approach while supporting the
Session distinction, so a similar solution is possible , but the user (programmer) must both keep a reference to
sessionmaker and carefully manage the lifetime of the
Session object, what is error prone. It should nevertheless be a feasible approach.
Waiting for the next version might make sense here, although it is of note that the minimum required version of
Python is bumped to 3.6 there  (not an issue for us as we are currently using 3.7.3, but might cause problems elsewhere). The migration document also promises various performance improvements in the next version.
The required changes to the code are non-trivial for both versions and the migration to v1.4 also seems to require some rewrites. I advise to do both at the same time and postpone this issue until after the
SQLAlchemy v1.4 release.
Updated by Radko Krkoš 8 months ago
As for the
flask-sqlalchemy, it is just a simple adaptor  and seems to take the
sessionmaker route . It also seems to manage the lifetime of the session subobject. Although it is not obvious from the example code  (why would an object's attribute lifetime be different from the object itself?), the documentation refers to the
session as scoped and declares :
You have to commit the session, but you don’t have to remove it at the end of the request, Flask-SQLAlchemy does that for you.
So the way of
flask-sqlalchemy should be a viable approach to correctly access the database and if not, it is a bug somewhere in the
psycopg2 stack and should be treated as such.
This is all based on code review and study of the documentation, not tested hard evidence.
Updated by Radko Krkoš 7 months ago
Further investigating the
flask-sqlalchemy, a simple testing script was prepared (attached), to determine the session management behaviour. The output, especially section "Commit - transaction start", shows that
commit() in fact does not immediately start another transaction and that is deferred until the next query. Therefore, a viable approach using
from flask import Flask from flask_sqlalchemy import SQLAlchemy app = Flask(__name__) db = SQLAlchemy(app) # New transaction db.session.query(...) db.session.commit() # New transaction db.session.query(...) db.session.commit() ...
as opposed to
rollback() that is required using the current architecture based on pure