flask-sqlalchemy icon indicating copy to clipboard operation
flask-sqlalchemy copied to clipboard

models_committed signal triggered too early with nested transactions

Open michamos opened this issue 7 years ago • 3 comments

I would like to be able to determine when changes have been committed to the db so that a query from a different worker will see the new data.

It looks like I could use the models_committed signal, but it doesn't seem to work properly when using SAVEPOINT transactions. In that case, the signal gets sent on the commit() of the nested transaction, rather than the real commit of the full transaction to the database.

This can be seen on the the following example:

from flask import Flask
from flask_sqlalchemy import SQLAlchemy, models_committed


app = Flask(__name__)
app.config['SQLALCHEMY_DATABASE_URI'] = 'sqlite:////tmp/test.db'
app.config['SQLALCHEMY_TRACK_MODIFICATIONS'] = True
db = SQLAlchemy(app)

class User(db.Model):
    id = db.Column(db.Integer, primary_key=True)
    username = db.Column(db.String(80), unique=True, nullable=False)
    email = db.Column(db.String(120), unique=True, nullable=False)


@models_committed.connect
def committed(*args, **kwargs):
    print('committed signal sent')


db.create_all()

with db.session.begin_nested():
    print('inside nested')
    admin = User(username='admin', email='[email protected]')
    db.session.add(admin)
    print('added admin')
print('outside nested')
db.session.commit()

The behavior I would expect is that the signal gets sent when doing the commit() on the last line, but instead the output is:

inside nested
added admin
committed signal sent
outside nested

due to the implicit commit() when exiting the context manager.

One solution would be to modify https://github.com/mitsuhiko/flask-sqlalchemy/blob/50944e77522d4aa005fc3c833b5a2042280686d3/flask_sqlalchemy/init.py#L212 in order to detect whether the current transaction is nested, and don't do anything in that case.

Is there any other way to reliably detect when a transaction has been committed?

michamos avatar Oct 10 '18 17:10 michamos

~~A simple solution is to replace the after_commit SQLAlchemy ORM event by after_transaction_end:~~

This event differs from after_commit() in that it corresponds to all SessionTransaction objects in use, including those for nested transactions and subtransactions, and is always matched by a corresponding after_transaction_create() event.

I am confused by this description. It looks like the after_commit SQLAlchemy event should only be triggered on real commits of the full transaction, as opposed to after_transaction_end, that should also be sent for nested transactions.

Edit: After checking in more detail, I found out that unlike the after_commit event which is triggered twice (once at the end of the nested transaction, once at the end of the full transaction), the after_transaction_end is triggered three times: twice at the end of the nested transaction (first with transaction.nested == False and then right after with transaction.nested == True) and once at the end of the full transaction (with transaction.nested == False).

michamos avatar Oct 11 '18 08:10 michamos

SQLAlchemy exposes after_transaction_end. Wouldn't it be better to rely on this signal to serve your use case ?

ticosax avatar Jan 21 '19 18:01 ticosax

That doesn't work, as that signal is emitted for every transaction end, not only the outer one. The solution adopted in #646 is to check explicitly if the transaction is nested or a subtransaction in all the different handlers.

michamos avatar Jun 06 '19 09:06 michamos

see close reason in associated PR #646

davidism avatar Sep 18 '22 16:09 davidism