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

Add support for nested sessions (SAVEPOINTs) in modifications tracking

Open barakalon opened this issue 9 years ago • 9 comments

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:

barakalon avatar Dec 04 '15 20:12 barakalon

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. :-)

davidism avatar Dec 11 '15 19:12 davidism

Thanks @davidism

I've been using this in production for about a month now and it's working fine.

barakalon avatar Jan 08 '16 00:01 barakalon

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 avatar Jan 08 '16 11:01 immunda

@immunda Just to make sure I'm on the same page, you mean SQLAlchemy's events, yes?

barakalon avatar Jan 09 '16 20:01 barakalon

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.

immunda avatar Jan 10 '16 10:01 immunda

Ah, right on.

Well when tracking modifications is disabled by default, support for savepoints will still be a good thing.

barakalon avatar Jan 10 '16 18:01 barakalon

Why did you close this?

davidism avatar Aug 29 '16 21:08 davidism

Ah, just because its been open for many months and I was doing a bit of house cleaning. I figured it was dead.

barakalon avatar Aug 29 '16 21:08 barakalon

Would be interesting to know how much this affects performance since per @immunda even the current naive implementation affects performance "adversely".

roganov avatar Sep 12 '16 20:09 roganov

#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.

davidism avatar Sep 18 '22 16:09 davidism