flask-sqlalchemy
flask-sqlalchemy copied to clipboard
Add support for nested sessions (SAVEPOINTs) in modifications tracking
I love the signals for tracking modifications to models. However, they don't yet support nested sessions.
Here is a proposal for adding support for this. It uses a stack of dictionaries instead of just a dictionary for tracking models that have been inserted/updated/deleted. When a nested session begins, a new dictionary is pushed to the stack. If that nested session is rolled back, the changes that happened during that session are removed. If that nested session is committed, the changes are merged into the next item.
Anywho, let me know what you think! This is my first attempt at a contribution to flask-sqlalchemy, so take it easy on me :grin:
Thanks, this looks interesting. I'll try to take a look at it soon, just commenting to let you know I'm aware of it. :-)
Thanks @davidism
I've been using this in production for about a month now and it's working fine.
Whilst this change in the current context looks good, there are plans to deprecate Flask-SQLAlchemy specific signals (#275), as SQLAlchemy signals have come a long way since this library was created and some of the signals are quite flawed. This is also inline with plans to not track modifications by default as it adversely affects performance.
Happy to have input on these decisions, especially @davidism as he's input on this thread.
@immunda Just to make sure I'm on the same page, you mean SQLAlchemy's events, yes?
Sorry, yes, events. Some Django parlance is hard to shake.
I also think you should actually ignore my previous comment regarding #275, since #364 now fixes problems that led to that decision.
Ah, right on.
Well when tracking modifications is disabled by default, support for savepoints will still be a good thing.
Why did you close this?
Ah, just because its been open for many months and I was doing a bit of house cleaning. I figured it was dead.
Would be interesting to know how much this affects performance since per @immunda even the current naive implementation affects performance "adversely".
#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.