feedback

Author: mbertrand

learning_resources/management/commands/backpopulate_mit_edx_data.py

@@ -62,6 +62,11 @@ def handle(self, *args, **options):  # noqa: ARG002
             self.stdout.write(
                 f"Population of MIT edX data finished, took {total_seconds} seconds"
             )
-            self.stdout.write(
-                f"Populated {count} resources. See celery logs for details."
-            )
+            if count is None:
+                self.stdout.write(
+                    "Task skipped (rate-limited). See celery logs for details."
+                )
+            else:
+                self.stdout.write(
+                    f"Populated {count} resources. See celery logs for details."
+                )

learning_resources/management/commands/backpopulate_mitxonline_data.py

@@ -48,6 +48,11 @@ def handle(self, *args, **options):  # noqa: ARG002
             self.stdout.write(
                 f"Population of MITX Online data finished, took {total_seconds} seconds"
             )
-            self.stdout.write(
-                f"Populated {count} resources. See celery logs for details."
-            )
+            if count is None:
+                self.stdout.write(
+                    "Task skipped (rate-limited). See celery logs for details."
+                )
+            else:
+                self.stdout.write(
+                    f"Populated {count} resources. See celery logs for details."
+                )

learning_resources/management/commands/backpopulate_oll_data.py

@@ -55,6 +55,11 @@ def handle(self, *args, **options):  # noqa: ARG002
             self.stdout.write(
                 f"Population of oll data finished, took {total_seconds} seconds"
             )
-            self.stdout.write(
-                f"Populated {count} resources. See celery logs for details."
-            )
+            if count is None:
+                self.stdout.write(
+                    "Task skipped (rate-limited). See celery logs for details."
+                )
+            else:
+                self.stdout.write(
+                    f"Populated {count} resources. See celery logs for details."
+                )

learning_resources/management/commands/backpopulate_xpro_data.py

@@ -49,6 +49,11 @@ def handle(self, *args, **options):  # noqa: ARG002
             self.stdout.write(
                 f"Population of xpro data finished, took {total_seconds} seconds"
             )
-            self.stdout.write(
-                f"Populated {count} resources. See celery logs for details."
-            )
+            if count is None:
+                self.stdout.write(
+                    "Task skipped (rate-limited). See celery logs for details."
+                )
+            else:
+                self.stdout.write(
+                    f"Populated {count} resources. See celery logs for details."
+                )

learning_resources/tasks.py

@@ -122,7 +122,7 @@ def get_micromasters_data():
 def get_mit_edx_data(
     api_course_datafile: str | None = None,
     api_program_datafile: str | None = None,
-) -> int:
+) -> int | None:
     """Task to sync MIT edX data with the database
 
     Args:
@@ -132,7 +132,8 @@ def get_mit_edx_data(
             Otherwise, the API is queried directly.
 
     Returns:
-        int: The number of results that were fetched
+        int | None: The number of results fetched, or None if the call was
+            skipped due to rate limiting.
     """
     courses = pipelines.mit_edx_courses_etl(api_course_datafile)
     programs = pipelines.mit_edx_programs_etl(api_program_datafile)
@@ -142,7 +143,7 @@ def get_mit_edx_data(
 
 @app.task
 @rate_limited_task(wait_time=900)
-def get_mitxonline_data() -> int:
+def get_mitxonline_data() -> int | None:
     """Execute the MITX Online ETL pipeline"""
     courses = pipelines.mitxonline_courses_etl()
     programs = pipelines.mitxonline_programs_etl()

main/decorators.py

@@ -10,8 +10,12 @@
 
 def rate_limited_task(wait_time: int, key: str | None = None):
     """
-    Drop calls made within `wait_time` seconds of the last successful invocation.
-    Uses an atomic `cache.add` so it is safe across Celery workers.
+    Drop calls made within `wait_time` seconds of the previous invocation.
+
+    The lock is acquired before the wrapped function runs and is not released
+    on exception — failures still count against the rate limit to prevent
+    retry storms against upstream APIs. Uses an atomic `cache.add` so it is
+    safe across Celery workers.
 
     Args:
         wait_time: Lock duration in seconds.

main/decorators_test.py

@@ -6,7 +6,7 @@
 def _patch_redis_cache(mocker):
     """Patch the redis cache used by rate_limited_task and return its mock."""
     mock_redis = mocker.Mock()
-    mocker.patch.dict("main.decorators.caches", {"redis": mock_redis})
+    mocker.patch("main.decorators.caches", {"redis": mock_redis})
     return mock_redis