Merge pull request #1164 from procrastinate-org/cron-django

Fix Django + cron using the wrong connector

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

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()

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,20 +144,20 @@ 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"
 
 # 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

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):

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

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))

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():