add on_commit decorator

[obayemi]

I recently had to add on_commit hooks with lambdas to some functions (mainly email notifications in post_save signals handler)
That was a lot of boilerplate so I wrote a simple decorator for making a function call only trigger on transaction commit

def call_on_commit(f):
    """
    only call the decorated function at transaction commit
    warning the return value will be ignored
    """
    def handle(*args, **kwargs):
        transaction.on_commit(lambda: f(*args, **kwargs))
    return handle

leading to

@call_on_commit
def send_message(user, context):
    # send message

instead of

def send_message(user, context):
    def _send_message():
        # send message
    on_commit(do_stuff)

I made a PR on github to implement this feature if it seems relevant
​https://github.com/django/django/pull/10167

[Carlton Gibson]

I think you need to explain the usage more fully here.

I'm expecting this sort of thing:

def send_mail()
    ...

with transaction.atomic():
    Thing.objects.create(...)
    transaction.on_commit(send_mail)

You need to explain how the decorator would work.

From the docs:

If you call on_commit() while there isn’t an active transaction, the callback will be executed immediately.

So you can't just decorate send_mail() at the point of declaration here. So what would your code look like?

[obayemi]

Replying to Carlton Gibson:

I think you need to explain the usage more fully here.

I'm expecting this sort of thing:

def send_mail()
    ...

with transaction.atomic():
    Thing.objects.create(...)
    transaction.on_commit(send_mail)

You need to explain how the decorator would work.

From the docs:

If you call on_commit() while there isn’t an active transaction, the callback will be executed immediately.

So you can't just decorate send_mail() at the point of declaration here. So what would your code look like?

The decorator don't run the function at the declaration, it only makes every usage run at transaction commit, and it would be completely transparent for the user as even if no transaction is active it would onlybe run synchronously as you stated from the doc
Manually calling on_commit(function) can work, but it adds some boilerplate if your function should always be run at the end of transaction, and it is easy to forget

The decorator would be usefull in two cases
1) when you have to automatically add every invocation of the function to the commit hook instead of running it instantly
2) when your function is ran by django e.g. post_save on creation when you did not finshed filling all the items in a m2m field

for example, one my actual use case is that I have callbacks that get called in post_save in a complex objects that needs a lot of data in related objects that with foreignkeys to the objects that was created
like if you wanted to send a message at every user in a group at creation with a post_save signal

with transaction.atomic():
    group = Group.objects.create()
    group.users.add(Users.objects.all()

You would have to do

@receiver(post_save, sender=Group)
def notify_users(sender, instance, created, *args, **kwargs):
    def _notify_users():  # defining inner function to remove the need to pass parameters from context since they can't be used in on_commit hook
        if created:
            for user in group.users.all():
                user.notify_group_creation(group)

    # without on_commit, there wouldn't be any users in the group yet
    on_commit(_notify_users)

where the goal of my decorator is to remove the need to declare an inner function and call it in the on_commit hook explicitely which is useless boilerplate and indent the whole function one step more if the whole function should be ran after the commit
using this decorator would allow only writing the receiver as

@receiver(post_save, sender=Group)
@call_on_commit
def notify_users(sender, instance, created, *args, **kwargs):
    if created:
        for user in group.users.all():
            user.notify_group_creation(group)

[Carlton Gibson]

Hmmm. OK. I see. My immediate thought there is that you would be MUCH better avoiding the post save signal and just using the on_commit hook straight-forwardly. The control flow would be significantly clearer.

I'm inclined towards closing this as wontfix on that basis. I'll leave some time for others to comment though.

[obayemi]

I agree that the workflow could be better without the post_save, but it ease the dev a lot IMHO
that way we can't forget to call the handler everywhere the Models can be created and it is simpler to think, (these will be always be called at creation, don't need to think about it (isn't this why signals are implemented anyway?))

[Carlton Gibson]

isn't this why signals are implemented anyway?

Not really. There's a good article on it here: ​Django Anti-Patterns: Signals

You really are better off explicitly calling the handler in the places you need it. (They'll be few in reality.)

However, that's an off-topic discussion for here... 🙂

[Carlton Gibson]

Hi @obayemi.

Having considered this more I'm going to go with wontfix.

You're obviously welcome to use a decorator such as this in your own project but, using on_commit() directly is going to result in significantly clearer code.

In your example, do this:

with transaction.atomic():
    group = Group.objects.create()
    group.users.add(Users.objects.all()
    transaction.on_commit(notify_users)

By shipping a call_on_commit decorator we'd be guiding people towards writing less clear code. I think we're not doing anyone any favours there.

Thanks for the input!