flask-sqlalchemy
flask-sqlalchemy copied to clipboard
pallets-eco
correctly handle signals in nested transactions
This PR fixes the issue reported in #645 by checking whether the current transaction is nested in the before_commit and after_commit handlers, and not doing anything if that's the case. Handling rollbacks of nested transaction is a bit more tricky: it requires maintaining a stack of the changes in every subtransaction, and only discarding the changes in the subtransactions that are rolled back.
I have added a test (and a fixture to work around issues in the sqlite driver) to check that the behavior (actual changes sent at the moment the full transaction is committed) is correct in two cases: when a subtransaction rollback preserves an outer transaction, and when it causes the whole transaction to be rolled back.
@davidism sorry to bother you again about this, but this is really an important issue for us, and we currently need to use ugly workarounds because of it (retrying tasks periodically until the real commit has happened and they get the new version when they query the DB). I am willing to work further on it and do whatever is needed to get it fixed. Could you maybe provide some insight into whether something like this (with potential modifications) could eventually be merged?
What's the plan for this after https://github.com/pallets/flask-sqlalchemy/pull/727 ? as there is no plan to deprecate the modification tracking any more but just disable it by default, I think this fix (or something similar) should be added, as it's currently broken when using begin_nested. Note we have been using this in production for a few months now with now adverse effects.
Duplicate of #358, an alternate implementation. Closing for the same reasons:
https://github.com/pallets-eco/flask-sqlalchemy/pull/1087 changes modification tracking, so this will no longer be mergeable. I also have the impression that how SQLAlchemy tracks nested transactions has changed, but I'm not sure. That said, after considering it more, I don't want to add more complexity to the track modifications feature. If you need other behavior, you're better off using SQLAlchemy's events directly.
