Should ENQUEUE_ON_COMMIT default to False?

[hakib]

In django_tasks.backends.database.DatabaseBackend there is a parameter ENQUEUE_ON_COMMIT to control whether tasks should enqueue in the same transaction, or after the transaction commits using transaction.on_commit. The default is to enqueue on commit - not in the same transaction.

Consider this example:

from django.db import transaction
from django_tasks import task

@task()
def send_email(to: str, message: str) -> None:
    print(f"Sending email to {to}: {message}")

def notify_user(order_id: int) -> Order:
    with transaction.atomic():
        order = Order.objects.select_for_update().get(id=order_id)
        if order.user_notified:
            return order
        # ↓↓↓ What happens if this fails? ↓↓↓
        send_email.enqueue(order.user.email, "Your order has been completed.")
        order.user_notified = True
        order.save()
    return order

If for some reason send_email.enqueue failed to enqueue (not execute! enqueue...):

• If enqueue happens in the same database transaction -> an exception will raise, the process will rollback, user_notified will remain False (good!)
• If enqueue happens on_commit -> an exception will raise, the function will fail, user_notified will be True (bad!)

The problem in the second scenario is that when enqueue failed, the transaction already committed and set user_notified to True. This is bad because the user was not, and will not, be notified which is inconsistent with what we just committed to the db.

I think a safer default would be False - enqueue in the same transaction.

[RealOrangeOne]

There were some discussions on the upstream PR about ENQUEUE_ON_COMMIT having a few gaps, although those were around multiple databases.

In this case, I agree it'd be better to enqueue in the same transaction. I think hindsight is showing me that it's complex enough to need much more configuration than is currently allowed, to the point it might just be worth removing the setting entirely and deferring it to the user.

[hakib]

Thanks for the quick reply. A few thoughts from the discussion on the pr:

1. I didn't even consider multi databases.
2. The comparison to celery seems irrelevant given it's incapable of enqueuing in the same database transaction
3. On the top my head, I can't think of a scenario where I wouldn't want to enqueue in the db transaction if possible, let alone having this as the default

Several commenters suggested ditching this option entirely for the first version as it is difficult to understand and adds complexity to the implementation. Instead, the experienced developer can easily use enqueue in an on_commit by himself, they don't really need the framework to do it for them. I think that's a good path forward.

I didn't realize there was already a discussion. Feel free to close this issue and continue the discussion wherever.