HiveBrain v1.2.0
Get Started
← Back to all entries
patternpythondjangoMinor

Replacement for commit_on_success

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
commit_on_successforreplacement

Problem

I am porting an application from django 1.4.5 to django 1.6.1. I found that commit_on_success has been deprecated. Moreover I found specific incompatibilities when calling get_or_create from inside commit_on_success.

atomic is often mentioned as drop-in replacement for commit_on_success, but using atomic alone does not always guaranteed durability, and there appear to be no official solution to that problem.

I have tried writing a replacement for commit_on_success by building on top of atomic and adding back in the durability guarantee. Does this code look like it solves the problem? Are there cases, where this code would fail to provide atomicity and durability?

# -*- coding: utf-8 -*-

try: from django.db.transaction import (Atomic, TransactionManagementError,
                                        get_connection)
except ImportError: from django.db.transaction import commit_on_success
else:
    class DurableAtomic(Atomic):
        def __enter__(self):
            if get_connection(self.using).in_atomic_block:
                raise TransactionManagementError(
                    "Cannot use DurableAtomic inside Atomic")
            super(DurableAtomic, self).__enter__()
    def commit_on_success(using=None):
        if callable(using):
            return DurableAtomic(None, False)(using)
        return DurableAtomic(using, False)


As suggested by Quentin I have written unit tests for the code:

```
# -- coding: utf-8 --

from django.contrib.auth.models import User
from django.db.transaction import atomic, TransactionManagementError
from core.commit_on_success import commit_on_success
from django.test import TransactionTestCase

def update_user(username):
User.objects.filter(username=username) \
.update(email=username + '@example.org')

def is_updated(username):
return '.org' in User.objects.get(username=username).email

class TestException(Exception):
pass

class TestValidation(TransactionTestCase):
def

Solution

-
Yes, this clever code solves your problem : you won't be able to nest commit_on_success or atomic under commit_on_success. You already know the caveats, but I'll repeat them anyway:

  • If you do nest a block under a commit_on_success, your code will consistently fail: easy to fix as long as this code path is executed in your tests.



  • It's always possible to commit manually using connection.commit(), maybe you're using a library that does weird things, say by having its own transaction manager.



  • It's easy to maliciously change DurableAtomic.



-
Please, please write tests for this, to see that you do get the correct behaviour with and without nesting, with and without exceptions.

-
Nested atomic blocks map conceptually well with one big transaction for the outer block and savepoints for the nested blocks. I agree with Kevin Christopher Henry in your SO answer about commit_on_success going away: your concept of "durability without atomicity" doesn't make much sense, I would prefer to rewrite the code if possible to avoid relying on the dubious behavior of commit_on_success.

-
I've indented your try/except to be PEP8-compliant:

# -*- coding: utf-8 -*-

try:
    from django.db.transaction import (Atomic, TransactionManagementError,
                                       get_connection)
except ImportError:
    from django.db.transaction import commit_on_success
else:
    class DurableAtomic(Atomic):
        def __enter__(self):
            if get_connection(self.using).in_atomic_block:
                raise TransactionManagementError(
                    "Cannot use DurableAtomic inside Atomic")
            super(DurableAtomic, self).__enter__()

    def commit_on_success(using=None):
        if callable(using):
            return DurableAtomic(None, False)(using)
        return DurableAtomic(using, False)

Code Snippets

# -*- coding: utf-8 -*-

try:
    from django.db.transaction import (Atomic, TransactionManagementError,
                                       get_connection)
except ImportError:
    from django.db.transaction import commit_on_success
else:
    class DurableAtomic(Atomic):
        def __enter__(self):
            if get_connection(self.using).in_atomic_block:
                raise TransactionManagementError(
                    "Cannot use DurableAtomic inside Atomic")
            super(DurableAtomic, self).__enter__()

    def commit_on_success(using=None):
        if callable(using):
            return DurableAtomic(None, False)(using)
        return DurableAtomic(using, False)

Context

StackExchange Code Review Q#53751, answer score: 7

Revisions (0)

No revisions yet.