From dd77d1ce4898cd8fab0d20f7ec92bc41500d834e Mon Sep 17 00:00:00 2001 From: Shinya Okano Date: Thu, 1 Oct 2026 00:57:32 +0900 Subject: [PATCH] Remove the deprecated TaskBroker.enqueue() TaskBroker.enqueue() was renamed to notify() in 0.5, and a broker that still overrode it was called with a DeprecationWarning saying it would be ignored in 0.6. This removes the alias and the backend's fallback to the override, as announced. A broker that overrides only enqueue() now fails in notify() like any other broker failure: the error is logged and the task stays in the database for the worker and the HTTP endpoints. Bundled brokers and the BROKER option are unaffected. --- CHANGELOG.md | 14 +++++++++ CONTRIBUTING.md | 2 +- django_database_task/backends.py | 34 +-------------------- django_database_task/brokers/base.py | 15 ---------- tests/test_brokers.py | 45 +++++++--------------------- 5 files changed, 27 insertions(+), 83 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index cda3924..dea8f45 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -90,6 +90,20 @@ changes. ([#34](https://github.com/tokibito/django-database-task/issues/34)) +### Removed + +- `TaskBroker.enqueue()`, deprecated in 0.5 and removed here as announced. A + broker that overrides it instead of `notify()` is no longer called: the + backend calls `notify()`, which raises `NotImplementedError` on + `TaskBroker`, and logs `Broker X failed to notify about task Y` at ERROR + for every task, as for any broker failure. The task is saved as before, so + `run_database_tasks` and the HTTP endpoints still pick it up, but the + external service is no longer told about it. Code that called + `broker.enqueue()` by hand gets an `AttributeError`; call `notify()`. + Bundled brokers and the `BROKER` option are unaffected, as are brokers + already on `notify()`; only a broker written by hand against 0.4 that was + not renamed, which has warned since 0.5, needs the change. + ### Fixed - A task could run twice when more than one worker polled the same queue. diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index ea4cae9..35c6645 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -160,7 +160,7 @@ Projects depend on the settings, the URLs and the management commands, so a change to any of them needs a path that keeps working for someone who upgrades without reading the release notes. Where something has to go, deprecate it first: keep it working with a `DeprecationWarning` that names the version it is -removed in, as `TaskBroker.enqueue()` does. +removed in, as `TaskBroker.enqueue()` did from 0.5 until its removal in 0.6. The tests are the safety net for this. When a refactor leaves the existing tests passing unmodified, that is the evidence that behaviour did not change. diff --git a/django_database_task/backends.py b/django_database_task/backends.py index dfa504c..1a0d729 100644 --- a/django_database_task/backends.py +++ b/django_database_task/backends.py @@ -2,7 +2,6 @@ import logging import time import traceback -import warnings from functools import cached_property from importlib import import_module from inspect import iscoroutinefunction @@ -75,11 +74,6 @@ class DatabaseTaskBackend(BaseTaskBackend): # option takes precedence over it. broker_class = None - # Set on an instance once a broker that still overrides enqueue() - # rather than notify() has been reported, so the deprecation warning is - # emitted once per backend. - _legacy_broker_enqueue_warned = False - def __init__(self, alias, params): super().__init__(alias, params) # Built eagerly so a misconfigured broker is reported when the @@ -119,7 +113,7 @@ def notify_broker(self, task_result): return try: - self._call_broker(task_result) + self.broker.notify(task_result) except Exception: logger.exception( "Broker %s failed to notify about task %s", @@ -127,32 +121,6 @@ def notify_broker(self, task_result): task_result.id, ) - def _call_broker(self, task_result): - """ - Call the broker, through a deprecated enqueue() override if it has - one, so a broker written against 0.4 keeps being told about tasks - rather than silently going quiet. - """ - # A broker need not subclass TaskBroker, so it may have no - # enqueue() at all; that is the new shape, not a legacy one. - implementation = getattr(type(self.broker), "enqueue", None) - if implementation is None or getattr( - implementation, "_is_library_notify", False - ): - return self.broker.notify(task_result) - - if not self._legacy_broker_enqueue_warned: - warnings.warn( - f"{type(self.broker).__name__}.enqueue() is deprecated and " - "will be ignored in django-database-task 0.6. Rename it to " - "notify().", - DeprecationWarning, - stacklevel=3, - ) - self._legacy_broker_enqueue_warned = True - - return self.broker.enqueue(task_result) - def get_auth_handlers(self, endpoint=None): """ Get the authentication handlers for the task HTTP endpoints. diff --git a/django_database_task/brokers/base.py b/django_database_task/brokers/base.py index 0095c46..6692273 100644 --- a/django_database_task/brokers/base.py +++ b/django_database_task/brokers/base.py @@ -63,21 +63,6 @@ def notify(self, task_result): """ raise NotImplementedError(f"{type(self).__name__} must implement notify().") - def enqueue(self, task_result): - """ - Notify the external service. - - .. deprecated:: 0.5 - Renamed to notify(). The backend still calls an enqueue() - a broker overrides, with a DeprecationWarning; that stops - working in 0.6. - """ - return self.notify(task_result) - - # Marks the implementation above as this library's, so a broker that - # overrides enqueue() can be told apart from one that inherits it. - enqueue._is_library_notify = True - def resolve_queue(self, queue_name): """ Translate a Django queue name into the broker's queue identifier. diff --git a/tests/test_brokers.py b/tests/test_brokers.py index bc5a711..52e6bb4 100644 --- a/tests/test_brokers.py +++ b/tests/test_brokers.py @@ -125,11 +125,11 @@ def test_saving_a_task_works_without_a_broker(self): @pytest.mark.django_db -class TestDeprecatedEnqueue: - """Tests for brokers written against the 0.4 method name.""" +class TestRemovedEnqueue: + """Tests for brokers written against the 0.4 method name, removed in 0.6.""" - def test_an_override_is_still_called_and_warns(self): - """A broker with enqueue() keeps being told, rather than going quiet.""" + def test_an_override_is_no_longer_called(self, caplog, recwarn): + """The failure to notify is logged, and the task stays in the database.""" notified = [] class LegacyBroker(TaskBroker): @@ -138,41 +138,18 @@ def enqueue(self, task_result): backend = make_backend(BROKER=LegacyBroker) - with pytest.warns(DeprecationWarning, match="enqueue"): - result = backend.enqueue(simple_task, (1, 2), {}) - - assert [r.id for r in notified] == [result.id] - - def test_the_warning_is_emitted_once_per_backend(self, recwarn): - class LegacyBroker(TaskBroker): - def enqueue(self, task_result): - pass - - backend = make_backend(BROKER=LegacyBroker) - backend.enqueue(simple_task, (1, 2), {}) - backend.enqueue(simple_task, (1, 2), {}) - - assert ( - len([w for w in recwarn.list if issubclass(w.category, DeprecationWarning)]) - == 1 - ) - - def test_a_broker_on_the_new_name_does_not_warn(self, recwarn): - backend = make_backend(BROKER=RecordingBroker) - - backend.enqueue(simple_task, (1, 2), {}) + result = backend.enqueue(simple_task, (1, 2), {}) + assert notified == [] + assert DatabaseTask.objects.get(id=result.id).status == "READY" + assert "LegacyBroker failed to notify about task" in caplog.text + assert "LegacyBroker must implement notify()" in caplog.text assert [ w for w in recwarn.list if issubclass(w.category, DeprecationWarning) ] == [] - def test_the_inherited_alias_still_works(self): - """Calling broker.enqueue() by hand reaches notify().""" - broker = RecordingBroker(backend=None) - - broker.enqueue("task-result") - - assert broker.notified == ["task-result"] + def test_the_alias_is_gone(self): + assert not hasattr(TaskBroker, "enqueue") class TestBrokerAuthHandlers: