fix: queue tasks in on_commit hook (#8149)

* fix: dispatch tasks in on_commit hook * test: fix test
ietf-tools · Nov 6, 2024 · afbe6aa · afbe6aa
1 parent 7b749f1
commit afbe6aa
Show file tree

Hide file tree

Showing 3 changed files with 25 additions and 7 deletions.
diff --git a/ietf/community/models.py b/ietf/community/models.py
@@ -3,7 +3,7 @@
 
 
 from django.conf import settings
-from django.db import models
+from django.db import models, transaction
 from django.db.models import signals
 from django.urls import reverse as urlreverse
 
@@ -117,7 +117,10 @@ def notify_events(sender, instance, **kwargs):
     # start a Celery task during tests. To prevent this, don't queue a celery task if we're running
     # tests.
     if settings.SERVER_MODE != "test":
-        notify_event_to_subscribers_task.delay(event_id=instance.pk)
+        # Wrap in on_commit so the delayed task cannot start until the view is done with the DB
+        transaction.on_commit(
+            lambda: notify_event_to_subscribers_task.delay(event_id=instance.pk)
+        )
 
 
 signals.post_save.connect(notify_events)
diff --git a/ietf/community/tests.py b/ietf/community/tests.py
@@ -431,8 +431,10 @@ def test_subscription_for_group(self):
         r = self.client.get(url)
         self.assertEqual(r.status_code, 200)
 
+    # Mock out the on_commit call so we can tell whether the task was actually queued
+    @mock.patch("ietf.submit.views.transaction.on_commit", side_effect=lambda x: x())
     @mock.patch("ietf.community.models.notify_event_to_subscribers_task")
-    def test_notification_signal_receiver(self, mock_notify_task):
+    def test_notification_signal_receiver(self, mock_notify_task, mock_on_commit):
         """Saving a DocEvent should notify subscribers
         
         This implicitly tests that notify_events is hooked up to the post_save signal.

diff --git a/ietf/sync/views.py b/ietf/sync/views.py
@@ -8,6 +8,7 @@
 from django.conf import settings
 from django.contrib.auth.models import User
 from django.contrib.contenttypes.models import ContentType
+from django.db import transaction
 from django.http import HttpResponse, HttpResponseRedirect, Http404
 from django.shortcuts import render
 from django.utils import timezone
@@ -79,16 +80,28 @@ def notify(request, org, notification):
     if request.method == "POST":
         if notification == "index":
             log("Queuing RFC Editor index sync from notify view POST")
-            tasks.rfc_editor_index_update_task.delay()
+            # Wrap in on_commit so the delayed task cannot start until the view is done with the DB
+            transaction.on_commit(
+                lambda: tasks.rfc_editor_index_update_task.delay()
+            )
         elif notification == "queue":
             log("Queuing RFC Editor queue sync from notify view POST")
-            tasks.rfc_editor_queue_updates_task.delay()
+            # Wrap in on_commit so the delayed task cannot start until the view is done with the DB
+            transaction.on_commit(
+                lambda: tasks.rfc_editor_queue_updates_task.delay()
+            )
         elif notification == "changes":
             log("Queuing IANA changes sync from notify view POST")
-            tasks.iana_changes_update_task.delay()
+            # Wrap in on_commit so the delayed task cannot start until the view is done with the DB
+            transaction.on_commit(
+                lambda: tasks.iana_changes_update_task.delay()
+            )
         elif notification == "protocols":
             log("Queuing IANA protocols sync from notify view POST")
-            tasks.iana_protocols_update_task.delay()
+            # Wrap in on_commit so the delayed task cannot start until the view is done with the DB
+            transaction.on_commit(
+                lambda: tasks.iana_protocols_update_task.delay()
+            )
 
         return HttpResponse("OK", content_type="text/plain; charset=%s"%settings.DEFAULT_CHARSET)