From c9bb59a240c3c39ee492cdd6593b9cb77a963ef3 Mon Sep 17 00:00:00 2001 From: Rust Saiargaliev Date: Mon, 31 Oct 2022 15:43:11 +0100 Subject: [PATCH 1/3] Test scenario for #123 This commit itself will break test suite, so we can later address issue in #123. This bug was introduced in #103 where django_dramatiq started benefitting from `AppConfig.ready()` functionality. There was a research done in #100 explaining the underlying issue. --- tests/testapp1/apps.py | 3 +++ tests/testapp1/models.py | 2 ++ 2 files changed, 5 insertions(+) create mode 100644 tests/testapp1/models.py diff --git a/tests/testapp1/apps.py b/tests/testapp1/apps.py index b81b807..b5cb3c0 100644 --- a/tests/testapp1/apps.py +++ b/tests/testapp1/apps.py @@ -3,3 +3,6 @@ class Testapp1Config(AppConfig): name = "tests.testapp1" + + def ready(self): + from . import tasks diff --git a/tests/testapp1/models.py b/tests/testapp1/models.py new file mode 100644 index 0000000..5a95546 --- /dev/null +++ b/tests/testapp1/models.py @@ -0,0 +1,2 @@ +# Quite common use-case when you want to run tasks within models.py code +from tests.testapp1 import tasks From 1b938cf9da190a9a8b690aaad8aa911e4cb519b2 Mon Sep 17 00:00:00 2001 From: Rust Saiargaliev Date: Mon, 31 Oct 2022 15:58:39 +0100 Subject: [PATCH 2/3] Fix #123 -- init broker configuration before loading other app models --- django_dramatiq/apps.py | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/django_dramatiq/apps.py b/django_dramatiq/apps.py index d483200..44131fe 100644 --- a/django_dramatiq/apps.py +++ b/django_dramatiq/apps.py @@ -35,7 +35,21 @@ class DjangoDramatiqConfig(AppConfig): name = "django_dramatiq" verbose_name = "Django Dramatiq" + def import_models(self): + """ + Initialize Dramatiq configuration right after Django has loaded its models. + + Django calls `ready()` on all apps only after it has loaded all models. + Due to the fact that tasks often imported from models, we need to make sure + that Dramatiq is fully configured before that happens. + """ + super().import_models() + self.ready() + def ready(self): + if getattr(self, "_is_ready", False): + super().ready() + global RATE_LIMITER_BACKEND dramatiq.set_encoder(self.select_encoder()) @@ -76,6 +90,8 @@ def ready(self): broker = broker_class(middleware=middleware, **broker_options) dramatiq.set_broker(broker) + self._is_ready = True + @property def rate_limiter_backend(self): return type(self).get_rate_limiter_backend() From 7194a5f6fd07cdd6935c077dac3e871a7a7e7eee Mon Sep 17 00:00:00 2001 From: Rust Saiargaliev Date: Sat, 5 Nov 2022 09:25:15 +0100 Subject: [PATCH 3/3] Address review comments --- django_dramatiq/apps.py | 3 ++- tests/testapp1/apps.py | 2 +- tests/testapp1/models.py | 2 +- 3 files changed, 4 insertions(+), 3 deletions(-) diff --git a/django_dramatiq/apps.py b/django_dramatiq/apps.py index 44131fe..0cee071 100644 --- a/django_dramatiq/apps.py +++ b/django_dramatiq/apps.py @@ -48,7 +48,8 @@ def import_models(self): def ready(self): if getattr(self, "_is_ready", False): - super().ready() + return + super().ready() global RATE_LIMITER_BACKEND dramatiq.set_encoder(self.select_encoder()) diff --git a/tests/testapp1/apps.py b/tests/testapp1/apps.py index b5cb3c0..537f1a3 100644 --- a/tests/testapp1/apps.py +++ b/tests/testapp1/apps.py @@ -5,4 +5,4 @@ class Testapp1Config(AppConfig): name = "tests.testapp1" def ready(self): - from . import tasks + from . import tasks # noqa diff --git a/tests/testapp1/models.py b/tests/testapp1/models.py index 5a95546..be6fb96 100644 --- a/tests/testapp1/models.py +++ b/tests/testapp1/models.py @@ -1,2 +1,2 @@ # Quite common use-case when you want to run tasks within models.py code -from tests.testapp1 import tasks +from tests.testapp1 import tasks # noqa