From 66ac98e0f24f5a6b2531b819533bc63109c34728 Mon Sep 17 00:00:00 2001 From: Joachim Jablon Date: Fri, 16 Aug 2024 17:40:10 +0200 Subject: [PATCH] Fix Django + cron using the wrong connector --- docs/howto/advanced/sync_defer.md | 18 ------------- docs/howto/django/scripts.md | 4 +-- docs/howto/django/tests.md | 22 ++++++++-------- docs/howto/production/testing.md | 4 +-- procrastinate/app.py | 26 ++++++++++++++----- .../management/commands/procrastinate.py | 10 +++++-- .../integration/contrib/django/test_models.py | 14 +++++----- 7 files changed, 50 insertions(+), 48 deletions(-) diff --git a/docs/howto/advanced/sync_defer.md b/docs/howto/advanced/sync_defer.md index 4f493d1b0..629cfb4b9 100644 --- a/docs/howto/advanced/sync_defer.md +++ b/docs/howto/advanced/sync_defer.md @@ -55,24 +55,6 @@ app = App(connector=SQLAlchemyPsycopg2Connector()) app.open(engine) ``` -## Having multiple apps - -If you need to have multiple connectors interact with the tasks, you can -create multiple synchronized apps with {py:meth}`App.with_connector`: - -``` -import procrastinate - - -app = procrastinate.App( - connector=procrastinate.PsycopgConnector(...), -) - -sync_app = app.with_connector( - connector=procrastinate.SyncPsycopgConnector(...), -) -``` - ## Procrastinate's automatic connector selection Async connectors are able to summon their synchronous counterpart when needed diff --git a/docs/howto/django/scripts.md b/docs/howto/django/scripts.md index 57b0aea04..576d55e12 100644 --- a/docs/howto/django/scripts.md +++ b/docs/howto/django/scripts.md @@ -21,8 +21,8 @@ def main(): django.setup() # By default, the app uses the Django database connection, which is unsuitable # for the worker. - app = app.with_connector(app.connector.get_worker_connector()) - app.run_worker() + with app.replace_connector(app.connector.get_worker_connector()): + app.run_worker() if __name__ == "__main__": main() diff --git a/docs/howto/django/tests.md b/docs/howto/django/tests.md index dd642c366..37dd9e266 100644 --- a/docs/howto/django/tests.md +++ b/docs/howto/django/tests.md @@ -24,8 +24,8 @@ def app(): # Replace the connector in the current app # Note that this fixture gives you the app back for convenience, but it's # the same instance as you'd get with `procrastinate.contrib.django.app`. - with procrastinate_app.current_app.replace_connector(in_memory) as app_with_connector: - yield app_with_connector + with procrastinate_app.current_app.replace_connector(in_memory) as app: + yield app def test_my_task(app): # Run the task @@ -126,8 +126,8 @@ class TestingTaskClass(TransactionTestCase): my_task.defer(a=1, b=2) # Start worker - app = app.with_connector(app.connector.get_worker_connector()) - app.run_worker(wait=False, install_signal_handlers=False, listen_notify=False) + with app.replace_connector(app.connector.get_worker_connector()) + app.run_worker(wait=False, install_signal_handlers=False, listen_notify=False) # Check task has been executed assert ProcrastinateJob.objects.filter(task_name="my_task").status == "succeeded" @@ -144,8 +144,8 @@ def test_task(): my_task.defer(a=1, b=2) # Start worker - app = app.with_connector(app.connector.get_worker_connector()) - app.run_worker(wait=False, install_signal_handlers=False, listen_notify=False) + with app.replace_connector(app.connector.get_worker_connector()) + app.run_worker(wait=False, install_signal_handlers=False, listen_notify=False) # Check task has been executed assert ProcrastinateJob.objects.filter(task_name="my_task").status == "succeeded" @@ -153,11 +153,11 @@ def test_task(): # Or with a fixture @pytest.fixture def worker(transactional_db): - def _(): - app = app.with_connector(app.connector.get_worker_connector()) - app.run_worker(wait=False, install_signal_handlers=False, listen_notify=False) - return app - return _ + with app.replace_connector(app.connector.get_worker_connector()) + def f(): + app.run_worker(wait=False, install_signal_handlers=False, listen_notify=False) + return app + yield f def test_task(worker): # Run tasks diff --git a/docs/howto/production/testing.md b/docs/howto/production/testing.md index d9699ecbf..418735a3d 100644 --- a/docs/howto/production/testing.md +++ b/docs/howto/production/testing.md @@ -17,8 +17,8 @@ def app(): # Replace the connector in the current app # Note that this fixture gives you the app back for covenience, # but it's the same instance as `my_app`. - with my_app.replace_connector(in_memory) as app_with_connector: - yield app_with_connector + with my_app.replace_connector(in_memory) as app: + yield app def test_my_task(app): diff --git a/procrastinate/app.py b/procrastinate/app.py index a29ee6880..072aa5738 100644 --- a/procrastinate/app.py +++ b/procrastinate/app.py @@ -94,12 +94,18 @@ def __init__( self._register_builtin_tasks() - def with_connector(self, connector: connector_module.BaseConnector) -> App: + def with_connector( + self, + connector: connector_module.BaseConnector, + ) -> App: """ Create another app instance sychronized with this one, with a different - connector. For all things regarding periodic tasks, the original app - (and its original connector) will be used, even when the new app's - methods are used. + connector. + + .. deprecated:: 2.14.0 + Use `replace_connector` instead. Because this method creates a new + app that references the same tasks, and the task have a link + back to the app, using this method can lead to unexpected behavior. Parameters ---------- @@ -109,7 +115,7 @@ def with_connector(self, connector: connector_module.BaseConnector) -> App: Returns ------- : - A new compatible app. + A new app with the same tasks. """ app = App( connector=connector, @@ -126,6 +132,12 @@ def replace_connector( ) -> Iterator[App]: """ Replace the connector of the app while in the context block, then restore it. + The context variable is the same app as this method is called on. + + >>> with app.replace_connector(new_connector) as app2: + ... ... + ... # app and app2 are the same object + Parameters ---------- @@ -134,8 +146,8 @@ def replace_connector( Yields ------- - `App` - A new compatible app. + : + A context manager that yields the same app with the new connector. """ old_connector = self.connector self.connector = connector diff --git a/procrastinate/contrib/django/management/commands/procrastinate.py b/procrastinate/contrib/django/management/commands/procrastinate.py index 2a66f2637..22bf59f17 100644 --- a/procrastinate/contrib/django/management/commands/procrastinate.py +++ b/procrastinate/contrib/django/management/commands/procrastinate.py @@ -1,6 +1,7 @@ from __future__ import annotations import asyncio +import contextlib from django.core.management.base import BaseCommand @@ -24,6 +25,11 @@ def add_arguments(self, parser): def handle(self, *args, **kwargs): kwargs = {k: v for k, v in kwargs.items() if k not in self._django_options} + context = contextlib.nullcontext() + if isinstance(app.connector, django_connector.DjangoConnector): - kwargs["app"] = app.with_connector(app.connector.get_worker_connector()) - asyncio.run(cli.execute_command(kwargs)) + kwargs["app"] = app + context = app.replace_connector(app.connector.get_worker_connector()) + + with context: + asyncio.run(cli.execute_command(kwargs)) diff --git a/tests/integration/contrib/django/test_models.py b/tests/integration/contrib/django/test_models.py index 022c81017..49615d51c 100644 --- a/tests/integration/contrib/django/test_models.py +++ b/tests/integration/contrib/django/test_models.py @@ -111,12 +111,14 @@ def my_task(timestamp): pass django_app = procrastinate.contrib.django.app - app = django_app.with_connector(django_app.connector.get_worker_connector()) - async with app.open_async(): - try: - await asyncio.wait_for(app.run_worker_async(), timeout=0.1) - except asyncio.TimeoutError: - pass + with django_app.replace_connector( + django_app.connector.get_worker_connector() + ) as app: + async with app.open_async(): + try: + await asyncio.wait_for(app.run_worker_async(), timeout=0.1) + except asyncio.TimeoutError: + pass periodic_defers = [] async for element in models.ProcrastinatePeriodicDefer.objects.values().all():