On 22 sept. 2013, at 19:38, Anssi Kääriäinen <anssi.kaariai...@thl.fi> wrote:
> There is no explicit use of savepoint=False. It is reasonable to expect that > the code succeeds on databases that allow this coding pattern. Yes, you're right… I looked at your version of the patch: 1) I believe the low-level APIs (rollback and savepoint_rollback) shouldn't be aware of the high level API (atomic), eg. they shouldn't manipulate the needs_rollback flag. The user should explicitly manipulate the flag with set_rollback if he starts messing with the low level APIs. I know the low-level APIs are calling `validate_no_atomic_block`, but that's a bit different. It's making a check and possibly raising an exception, not changing the internal state of the high level API. 2) You had to do many changes to the tests; it looks like some weren't very well written and your changes are justified. I'd like to double check. Thanks your the additional test cases, too. I'll write an iteration of the patch tonight, hopefully we'll converge to something we both consider mergeable. > So, as is, this API is bad. I appreciate the nuance in this assessment ;-) > If running read-only queries in marked-for-rollback blocks is absolutely > wanted, then lets just switch model.save() and other similar ORM operations > to use real savepoints. This is expensive for some use cases, but adding a > flag to avoid that cost can be done later on. The tx API is harder to change > after 1.6 release. I'm afraid DBAs will lynch us if we do that. -- Aymeric. -- You received this message because you are subscribed to the Google Groups "Django developers" group. To unsubscribe from this group and stop receiving emails from it, send an email to django-developers+unsubscr...@googlegroups.com. To post to this group, send email to django-developers@googlegroups.com. Visit this group at http://groups.google.com/group/django-developers. For more options, visit https://groups.google.com/groups/opt_out.
