Refs #35859 -- Removed support for Task enqueuing on transaction commit.

This removes the ability to configure Task enqueueing via a setting,
since the proposed `ENQUEUE_ON_COMMIT` did not support multi-database
setups.

Thanks to Simon Charette for the report.

Follow-up to 4289966d1b8e848e5e460b7c782dac009d746b20.

django/tasks/backends/base.py

@@ -4,8 +4,6 @@ from inspect import iscoroutinefunction
 from asgiref.sync import sync_to_async
 
 from django.conf import settings
-from django.core import checks
-from django.db import connections
 from django.tasks import DEFAULT_TASK_QUEUE_NAME
 from django.tasks.base import (
     DEFAULT_TASK_PRIORITY,
@@ -39,16 +37,8 @@ class BaseTaskBackend(metaclass=ABCMeta):
     def __init__(self, alias, params):
         self.alias = alias
         self.queues = set(params.get("QUEUES", [DEFAULT_TASK_QUEUE_NAME]))
-        self.enqueue_on_commit = bool(params.get("ENQUEUE_ON_COMMIT", True))
         self.options = params.get("OPTIONS", {})
 
-    def _get_enqueue_on_commit_for_task(self, task):
-        return (
-            task.enqueue_on_commit
-            if task.enqueue_on_commit is not None
-            else self.enqueue_on_commit
-        )
-
     def validate_task(self, task):
         """
         Determine whether the provided Task can be executed by the backend.
@@ -119,20 +109,4 @@ class BaseTaskBackend(metaclass=ABCMeta):
         )
 
     def check(self, **kwargs):
-        if self.enqueue_on_commit and not connections._settings:
+        return []
-            yield checks.Error(
-                "ENQUEUE_ON_COMMIT cannot be used when no databases are configured.",
-                hint="Set ENQUEUE_ON_COMMIT to False",
-                id="tasks.E001",
-            )
-
-        elif (
-            self.enqueue_on_commit
-            and not connections["default"].features.supports_transactions
-        ):
-            yield checks.Error(
-                "ENQUEUE_ON_COMMIT cannot be used on a database which doesn't support "
-                "transactions.",
-                hint="Set ENQUEUE_ON_COMMIT to False",
-                id="tasks.E002",
-            )

django/tasks/backends/dummy.py

@@ -1,7 +1,5 @@
 from copy import deepcopy
-from functools import partial
 
-from django.db import transaction
 from django.tasks.base import TaskResult, TaskResultStatus
 from django.tasks.exceptions import TaskResultDoesNotExist
 from django.tasks.signals import task_enqueued
@@ -43,10 +41,7 @@ class DummyBackend(BaseTaskBackend):
             worker_ids=[],
         )
 
-        if self._get_enqueue_on_commit_for_task(task) is not False:
+        self._store_result(result)
-            transaction.on_commit(partial(self._store_result, result))
-        else:
-            self._store_result(result)
 
         # Copy the task to prevent mutation issues.
         return deepcopy(result)

django/tasks/backends/immediate.py

@@ -1,8 +1,6 @@
 import logging
-from functools import partial
 from traceback import format_exception
 
-from django.db import transaction
 from django.tasks.base import TaskContext, TaskError, TaskResult, TaskResultStatus
 from django.tasks.signals import task_enqueued, task_finished, task_started
 from django.utils import timezone
@@ -92,9 +90,6 @@ class ImmediateBackend(BaseTaskBackend):
             worker_ids=[],
         )
 
-        if self._get_enqueue_on_commit_for_task(task) is not False:
+        self._execute_task(task_result)
-            transaction.on_commit(partial(self._execute_task, task_result))
-        else:
-            self._execute_task(task_result)
 
         return task_result

django/tasks/base.py

@@ -48,10 +48,6 @@ class Task:
     queue_name: str
     run_after: Optional[datetime]  # The earliest this Task will run.
 
-    # Whether the Task will be enqueued when the current transaction commits,
-    # immediately, or whatever the backend decides.
-    enqueue_on_commit: Optional[bool]
-
     # Whether the Task receives the Task context when executed.
     takes_context: bool = False
 
@@ -140,7 +136,6 @@ def task(
     priority=DEFAULT_TASK_PRIORITY,
     queue_name=DEFAULT_TASK_QUEUE_NAME,
     backend=DEFAULT_TASK_BACKEND_ALIAS,
-    enqueue_on_commit=None,
     takes_context=False,
 ):
     from . import task_backends
@@ -151,7 +146,6 @@ def task(
             func=f,
             queue_name=queue_name,
             backend=backend,
-            enqueue_on_commit=enqueue_on_commit,
             takes_context=takes_context,
             run_after=None,
         )

docs/ref/checks.txt

@@ -597,14 +597,6 @@ Signals
   a lazy reference to the sender ``<app label>.<model>``, but app
   ``<app label>`` isn't installed or doesn't provide model ``<model>``.
 
-Tasks
------
-
-* **tasks.E001**: ``ENQUEUE_ON_COMMIT`` cannot be used when no databases are
-  configured.
-* **tasks.E002**: ``ENQUEUE_ON_COMMIT`` cannot be used on a database which
-  doesn't support transactions.
-
 Templates
 ---------
 

docs/ref/settings.txt

@@ -2806,20 +2806,6 @@ You can use a backend that doesn't ship with Django by setting
 :setting:`BACKEND <TASKS-BACKEND>` to a fully-qualified path of a backend
 class (i.e. ``mypackage.backends.whatever.WhateverBackend``).
 
-.. setting:: TASKS-ENQUEUE_ON_COMMIT
-
-``ENQUEUE_ON_COMMIT``
-~~~~~~~~~~~~~~~~~~~~~
-
-Default: ``True``
-
-Whether to enqueue a Task only after the current transaction, if any, commits
-successfully, instead of enqueueing immediately.
-
-This can also be configured on a per-Task basis.
-
-See :ref:`Task transactions <task-transactions>` for more information.
-
 .. setting:: TASKS-QUEUES
 
 ``QUEUES``

docs/ref/signals.txt

@@ -717,9 +717,7 @@ Signals sent by the :doc:`tasks </ref/tasks>` framework.
 .. data:: django.tasks.signals.task_enqueued
    :module:
 
-Sent once a Task has been enqueued. If
+Sent once a Task has been enqueued.
-:attr:`django.tasks.Task.enqueue_on_commit` is set, the signal is only sent
-once the transaction commits successfully.
 
 Arguments sent with this signal:
 

docs/ref/tasks.txt

@@ -13,7 +13,7 @@ Task definition
 The ``task`` decorator
 ----------------------
 
-.. function:: task(*, priority=0, queue_name="default", backend="default", enqueue_on_commit=None, takes_context=False)
+.. function:: task(*, priority=0, queue_name="default", backend="default", takes_context=False)
 
     The ``@task`` decorator defines a :class:`Task` instance. This has the
     following optional arguments:
@@ -24,8 +24,6 @@ The ``task`` decorator
       Defaults to ``"default"``.
     * ``backend``: Sets the :attr:`~Task.backend` of the ``Task``. Defaults to
       ``"default"``.
-    * ``enqueue_on_commit``: Sets :attr:`~Task.enqueue_on_commit` for the
-      ``Task``. Defaults to ``None``.
     * ``takes_context``: Controls whether the ``Task`` function accepts a
       :class:`TaskContext`. Defaults to ``False``. See :ref:`Task context
       <task-context>` for details.
@@ -77,14 +75,6 @@ The ``task`` decorator
         this feature. Otherwise,
         :exc:`~django.tasks.exceptions.InvalidTask` is raised.
 
-    .. attribute:: Task.enqueue_on_commit
-
-        Whether the ``Task`` should be enqueued when the transaction commits
-        successfully, or immediately. Defaults to :setting:`ENQUEUE_ON_COMMIT
-        <TASKS-ENQUEUE_ON_COMMIT>` for the backend.
-
-        See :ref:`Task transactions <task-transactions>` for more information.
-
     .. attribute:: Task.name
 
         The name of the function decorated with :func:`task`. This name is not
@@ -210,9 +200,6 @@ Task results
 
         The time when the ``Task`` was enqueued.
 
-        If :attr:`Task.enqueue_on_commit` was set, this is the time the
-        transaction committed.
-
     .. attribute:: TaskResult.started_at
 
         The time when the ``Task`` began execution, on its first attempt.

docs/topics/tasks.txt

@@ -67,13 +67,6 @@ To use it, set :setting:`BACKEND <TASKS-BACKEND>` to
 The :class:`.ImmediateBackend` may also be useful in tests, to bypass the need
 to run a real background worker in your tests.
 
-.. admonition:: ``ImmediateBackend`` and ``ENQUEUE_ON_COMMIT``
-
-    When :setting:`ENQUEUE_ON_COMMIT <TASKS-ENQUEUE_ON_COMMIT>` is ``False``,
-    the Task will be executed within the same transaction it was enqueued in.
-
-    See :ref:`Task transactions <task-transactions>` for more information.
-
 .. _dummy-task-backend:
 
 Dummy backend
@@ -182,7 +175,7 @@ decorator arguments::
     from django.tasks import task
 
 
-    @task(priority=2, queue_name="emails", enqueue_on_commit=True)
+    @task(priority=2, queue_name="emails")
     def email_users(emails, subject, message):
         return send_mail(
             subject=subject, message=message, from_email=None, recipient_list=emails
@@ -304,33 +297,35 @@ conversion.
 Transactions
 ------------
 
-By default, Tasks are enqueued after the current database transaction (if there
+For most backends, Tasks are run in a separate process, using a different
-is one) commits successfully (using :meth:`transaction.on_commit()
+database connection. When using a transaction, without waiting for it to
-<django.db.transaction.on_commit>`), rather than enqueueing immediately. For
+commit, workers could start to process a Task which uses objects it can't
-most backends, Tasks are run in a separate process, using a different database
+access yet.
-connection. Without waiting for the transaction to commit, workers could start
-to process a Task which uses objects it can't access yet.
-
-This behavior can be changed by changing the :setting:`TASKS-ENQUEUE_ON_COMMIT`
-setting for the backend, or for a specific Task using the ``enqueue_on_commit``
-parameter.
 
 For example, consider this simplified example::
 
     @task
-    def my_task():
+    def my_task(thing_num):
-        Thing.objects.get()
+        Thing.objects.get(num=thing_num)
 
 
     with transaction.atomic():
-        Thing.objects.create()
+        Thing.objects.create(num=1)
-        my_task.enqueue()
+        my_task.enqueue(thing_num=1)
+
+To prevent the scenario where ``my_task`` runs before the ``Thing`` is
+committed to the database, use :func:`transaction.on_commit()
+<django.db.transaction.on_commit>`, binding all arguments to
+:meth:`~django.tasks.Task.enqueue` via :func:`functools.partial`::
+
+    from functools import partial
+
+    from django.db import transaction
 
 
-If :setting:`ENQUEUE_ON_COMMIT <TASKS-ENQUEUE_ON_COMMIT>` is ``False``, then it
+    with transaction.atomic():
-is possible for ``my_task`` to run before the ``Thing`` is committed to the
+        Thing.objects.create(num=1)
-database, and the Task won't be able to see the created object within your
+        transaction.on_commit(partial(my_task.enqueue, thing_num=1))
-transaction.
 
 .. _task-results:
 

tests/tasks/tasks.py

@@ -54,16 +54,6 @@ def exit_task():
     exit(1)
 
 
-@task(enqueue_on_commit=True)
-def enqueue_on_commit_task():
-    pass
-
-
-@task(enqueue_on_commit=False)
-def never_enqueue_on_commit_task():
-    pass
-
-
 @task()
 def hang():
     """

tests/tasks/test_custom_backend.py

@@ -27,7 +27,6 @@ class CustomBackendNoEnqueue(BaseTaskBackend):
     TASKS={
         "default": {
             "BACKEND": f"{CustomBackend.__module__}.{CustomBackend.__qualname__}",
-            "ENQUEUE_ON_COMMIT": False,
             "OPTIONS": {"prefix": "PREFIX: "},
         },
         "no_enqueue": {

tests/tasks/test_dummy_backend.py

@@ -2,17 +2,11 @@ from typing import cast
 from unittest import mock
 
 from django.db import transaction
-from django.db.utils import ConnectionHandler
 from django.tasks import TaskResultStatus, default_task_backend, task_backends
 from django.tasks.backends.dummy import DummyBackend
 from django.tasks.base import Task
 from django.tasks.exceptions import InvalidTask, TaskResultDoesNotExist
-from django.test import (
+from django.test import SimpleTestCase, TransactionTestCase, override_settings
-    SimpleTestCase,
-    TransactionTestCase,
-    override_settings,
-    skipIfDBFeature,
-)
 
 from . import tasks as test_tasks
 
@@ -22,7 +16,6 @@ from . import tasks as test_tasks
         "default": {
             "BACKEND": "django.tasks.backends.dummy.DummyBackend",
             "QUEUES": [],
-            "ENQUEUE_ON_COMMIT": False,
         }
     }
 )
@@ -119,14 +112,6 @@ class DummyBackendTestCase(SimpleTestCase):
         with self.assertRaises(TaskResultDoesNotExist):
             await default_task_backend.aget_result("123")
 
-    def test_enqueue_on_commit(self):
-        self.assertIs(
-            default_task_backend._get_enqueue_on_commit_for_task(
-                test_tasks.enqueue_on_commit_task
-            ),
-            True,
-        )
-
     def test_enqueue_logs(self):
         with self.assertLogs("django.tasks", level="DEBUG") as captured_logs:
             result = test_tasks.noop_task.enqueue()
@@ -150,20 +135,6 @@ class DummyBackendTestCase(SimpleTestCase):
         errors = list(default_task_backend.check())
         self.assertEqual(len(errors), 0, errors)
 
-    @override_settings(
-        TASKS={
-            "default": {
-                "BACKEND": "django.tasks.backends.dummy.DummyBackend",
-                "ENQUEUE_ON_COMMIT": True,
-            }
-        }
-    )
-    @mock.patch("django.tasks.backends.base.connections", ConnectionHandler({}))
-    def test_enqueue_on_commit_with_no_databases(self):
-        self.assertIn(
-            "tasks.E001", {error.id for error in default_task_backend.check()}
-        )
-
     def test_takes_context(self):
         result = test_tasks.get_task_id.enqueue()
         self.assertEqual(result.status, TaskResultStatus.READY)
@@ -188,7 +159,6 @@ class DummyBackendTestCase(SimpleTestCase):
                 "default": {
                     "BACKEND": "django.tasks.backends.dummy.DummyBackend",
                     "QUEUES": ["queue-1"],
-                    "ENQUEUE_ON_COMMIT": False,
                 }
             }
         ):
@@ -207,7 +177,6 @@ class DummyBackendTestCase(SimpleTestCase):
                 "default": {
                     "BACKEND": "django.tasks.backends.dummy.DummyBackend",
                     "QUEUES": ["queue-1"],
-                    "ENQUEUE_ON_COMMIT": False,
                 }
             }
         ):
@@ -224,39 +193,10 @@ class DummyBackendTransactionTestCase(TransactionTestCase):
         TASKS={
             "default": {
                 "BACKEND": "django.tasks.backends.dummy.DummyBackend",
-                "ENQUEUE_ON_COMMIT": True,
             }
         }
     )
-    def test_wait_until_transaction_commit(self):
+    def test_doesnt_wait_until_transaction_commit_by_default(self):
-        self.assertIs(default_task_backend.enqueue_on_commit, True)
-        self.assertIs(
-            default_task_backend._get_enqueue_on_commit_for_task(test_tasks.noop_task),
-            True,
-        )
-
-        with transaction.atomic():
-            test_tasks.noop_task.enqueue()
-
-            self.assertEqual(len(default_task_backend.results), 0)
-
-        self.assertEqual(len(default_task_backend.results), 1)
-
-    @override_settings(
-        TASKS={
-            "default": {
-                "BACKEND": "django.tasks.backends.dummy.DummyBackend",
-                "ENQUEUE_ON_COMMIT": False,
-            }
-        }
-    )
-    def test_doesnt_wait_until_transaction_commit(self):
-        self.assertIs(default_task_backend.enqueue_on_commit, False)
-        self.assertIs(
-            default_task_backend._get_enqueue_on_commit_for_task(test_tasks.noop_task),
-            False,
-        )
-
         with transaction.atomic():
             result = test_tasks.noop_task.enqueue()
 
@@ -265,73 +205,3 @@ class DummyBackendTransactionTestCase(TransactionTestCase):
             self.assertEqual(len(default_task_backend.results), 1)
 
         self.assertEqual(len(default_task_backend.results), 1)
-
-    @override_settings(
-        TASKS={
-            "default": {
-                "BACKEND": "django.tasks.backends.dummy.DummyBackend",
-            }
-        }
-    )
-    def test_wait_until_transaction_by_default(self):
-        self.assertIs(default_task_backend.enqueue_on_commit, True)
-        self.assertIs(
-            default_task_backend._get_enqueue_on_commit_for_task(test_tasks.noop_task),
-            True,
-        )
-
-        with transaction.atomic():
-            result = test_tasks.noop_task.enqueue()
-
-            self.assertIsNone(result.enqueued_at)
-
-            self.assertEqual(len(default_task_backend.results), 0)
-
-        self.assertEqual(len(default_task_backend.results), 1)
-        self.assertIsNone(result.enqueued_at)
-        result.refresh()
-        self.assertIsNotNone(result.enqueued_at)
-
-    @override_settings(
-        TASKS={
-            "default": {
-                "BACKEND": "django.tasks.backends.dummy.DummyBackend",
-                "ENQUEUE_ON_COMMIT": False,
-            }
-        }
-    )
-    def test_task_specific_enqueue_on_commit(self):
-        self.assertIs(default_task_backend.enqueue_on_commit, False)
-        self.assertIs(test_tasks.enqueue_on_commit_task.enqueue_on_commit, True)
-        self.assertIs(
-            default_task_backend._get_enqueue_on_commit_for_task(
-                test_tasks.enqueue_on_commit_task
-            ),
-            True,
-        )
-
-        with transaction.atomic():
-            result = test_tasks.enqueue_on_commit_task.enqueue()
-
-            self.assertIsNone(result.enqueued_at)
-
-            self.assertEqual(len(default_task_backend.results), 0)
-
-        self.assertEqual(len(default_task_backend.results), 1)
-        self.assertIsNone(result.enqueued_at)
-        result.refresh()
-        self.assertIsNotNone(result.enqueued_at)
-
-    @override_settings(
-        TASKS={
-            "default": {
-                "BACKEND": "django.tasks.backends.dummy.DummyBackend",
-                "ENQUEUE_ON_COMMIT": True,
-            }
-        }
-    )
-    @skipIfDBFeature("supports_transactions")
-    def test_enqueue_on_commit_with_no_transactions(self):
-        self.assertIn(
-            "tasks.E002", {error.id for error in default_task_backend.check()}
-        )

tests/tasks/test_immediate_backend.py

@@ -13,7 +13,6 @@ from . import tasks as test_tasks
         "default": {
             "BACKEND": "django.tasks.backends.immediate.ImmediateBackend",
             "QUEUES": [],
-            "ENQUEUE_ON_COMMIT": False,
         }
     }
 )
@@ -203,14 +202,6 @@ class ImmediateBackendTestCase(SimpleTestCase):
                 test_tasks.failing_task_value_error.using(run_after=timezone.now())
             )
 
-    def test_enqueue_on_commit(self):
-        self.assertIs(
-            default_task_backend._get_enqueue_on_commit_for_task(
-                test_tasks.enqueue_on_commit_task
-            ),
-            True,
-        )
-
     def test_enqueue_logs(self):
         with self.assertLogs("django.tasks", level="DEBUG") as captured_logs:
             result = test_tasks.noop_task.enqueue()
@@ -256,7 +247,6 @@ class ImmediateBackendTestCase(SimpleTestCase):
                 "default": {
                     "BACKEND": "django.tasks.backends.immediate.ImmediateBackend",
                     "QUEUES": ["queue-1"],
-                    "ENQUEUE_ON_COMMIT": False,
                 }
             }
         ):
@@ -275,7 +265,6 @@ class ImmediateBackendTestCase(SimpleTestCase):
                 "default": {
                     "BACKEND": "django.tasks.backends.immediate.ImmediateBackend",
                     "QUEUES": ["queue-1"],
-                    "ENQUEUE_ON_COMMIT": False,
                 }
             }
         ):
@@ -292,43 +281,10 @@ class ImmediateBackendTransactionTestCase(TransactionTestCase):
         TASKS={
             "default": {
                 "BACKEND": "django.tasks.backends.immediate.ImmediateBackend",
-                "ENQUEUE_ON_COMMIT": True,
             }
         }
     )
-    def test_wait_until_transaction_commit(self):
+    def test_doesnt_wait_until_transaction_commit_by_default(self):
-        self.assertIs(default_task_backend.enqueue_on_commit, True)
-        self.assertIs(
-            default_task_backend._get_enqueue_on_commit_for_task(test_tasks.noop_task),
-            True,
-        )
-
-        with transaction.atomic():
-            result = test_tasks.noop_task.enqueue()
-
-            self.assertIsNone(result.enqueued_at)
-            self.assertEqual(result.attempts, 0)
-            self.assertEqual(result.status, TaskResultStatus.READY)
-
-        self.assertEqual(result.status, TaskResultStatus.SUCCESSFUL)
-        self.assertIsNotNone(result.enqueued_at)
-        self.assertEqual(result.attempts, 1)
-
-    @override_settings(
-        TASKS={
-            "default": {
-                "BACKEND": "django.tasks.backends.immediate.ImmediateBackend",
-                "ENQUEUE_ON_COMMIT": False,
-            }
-        }
-    )
-    def test_doesnt_wait_until_transaction_commit(self):
-        self.assertIs(default_task_backend.enqueue_on_commit, False)
-        self.assertIs(
-            default_task_backend._get_enqueue_on_commit_for_task(test_tasks.noop_task),
-            False,
-        )
-
         with transaction.atomic():
             result = test_tasks.noop_task.enqueue()
 
@@ -337,51 +293,3 @@ class ImmediateBackendTransactionTestCase(TransactionTestCase):
             self.assertEqual(result.status, TaskResultStatus.SUCCESSFUL)
 
         self.assertEqual(result.status, TaskResultStatus.SUCCESSFUL)
-
-    @override_settings(
-        TASKS={
-            "default": {
-                "BACKEND": "django.tasks.backends.immediate.ImmediateBackend",
-            }
-        }
-    )
-    def test_wait_until_transaction_by_default(self):
-        self.assertIs(default_task_backend.enqueue_on_commit, True)
-        self.assertIs(
-            default_task_backend._get_enqueue_on_commit_for_task(test_tasks.noop_task),
-            True,
-        )
-
-        with transaction.atomic():
-            result = test_tasks.noop_task.enqueue()
-
-            self.assertIsNone(result.enqueued_at)
-            self.assertEqual(result.status, TaskResultStatus.READY)
-
-        self.assertEqual(result.status, TaskResultStatus.SUCCESSFUL)
-
-    @override_settings(
-        TASKS={
-            "default": {
-                "BACKEND": "django.tasks.backends.immediate.ImmediateBackend",
-                "ENQUEUE_ON_COMMIT": False,
-            }
-        }
-    )
-    def test_task_specific_enqueue_on_commit(self):
-        self.assertIs(default_task_backend.enqueue_on_commit, False)
-        self.assertIs(test_tasks.enqueue_on_commit_task.enqueue_on_commit, True)
-        self.assertIs(
-            default_task_backend._get_enqueue_on_commit_for_task(
-                test_tasks.enqueue_on_commit_task
-            ),
-            True,
-        )
-
-        with transaction.atomic():
-            result = test_tasks.enqueue_on_commit_task.enqueue()
-
-            self.assertIsNone(result.enqueued_at)
-            self.assertEqual(result.status, TaskResultStatus.READY)
-
-        self.assertEqual(result.status, TaskResultStatus.SUCCESSFUL)

tests/tasks/test_tasks.py

@@ -29,11 +29,9 @@ from . import tasks as test_tasks
         "default": {
             "BACKEND": "django.tasks.backends.dummy.DummyBackend",
             "QUEUES": ["default", "queue_1"],
-            "ENQUEUE_ON_COMMIT": False,
         },
         "immediate": {
             "BACKEND": "django.tasks.backends.immediate.ImmediateBackend",
-            "ENQUEUE_ON_COMMIT": False,
             "QUEUES": [],
         },
         "missing": {"BACKEND": "does.not.exist"},