Merge pull request #13773 from edx/efischer/actually_retry

update_course_in_cache should retry on exceptions

cms/djangoapps/contentstore/tests/test_import.py

@@ -9,6 +9,7 @@ from django.test.utils import override_settings
 from django.conf import settings
 import ddt
 import copy
+from mock import patch
 
 from openedx.core.djangoapps.content.course_structures.tests import SignalDisconnectTestMixin
 from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase
@@ -39,6 +40,14 @@ class ContentStoreImportTest(SignalDisconnectTestMixin, ModuleStoreTestCase):
         self.client = Client()
         self.client.login(username=self.user.username, password=self.user_password)
 
+        # block_structure.update_course_in_cache cannot succeed in tests, as it needs to be run async on an lms worker
+        self.task_patcher = patch('openedx.core.djangoapps.content.block_structure.tasks.update_course_in_cache')
+        self._mock_lms_task = self.task_patcher.start()
+
+    def tearDown(self):
+        self.task_patcher.stop()
+        super(ContentStoreImportTest, self).tearDown()
+
     def load_test_import_course(self, target_id=None, create_if_not_present=True, module_store=None):
         '''
         Load the standard course used to test imports

openedx/core/djangoapps/content/block_structure/tasks.py

@@ -15,9 +15,17 @@ log = logging.getLogger('edx.celery.task')
     default_retry_delay=settings.BLOCK_STRUCTURES_SETTINGS['BLOCK_STRUCTURES_TASK_DEFAULT_RETRY_DELAY'],
     max_retries=settings.BLOCK_STRUCTURES_SETTINGS['BLOCK_STRUCTURES_TASK_MAX_RETRIES'],
 )
-def update_course_in_cache(course_key):
+def update_course_in_cache(course_id):
     """
     Updates the course blocks (in the database) for the specified course.
     """
-    course_key = CourseKey.from_string(course_key)
-    api.update_course_in_cache(course_key)
+    try:
+        course_key = CourseKey.from_string(course_id)
+        api.update_course_in_cache(course_key)
+    except Exception as exc:   # pylint: disable=broad-except
+        # TODO: TNL-5799, check splunk logs to narrow down the broad except above
+        log.info("update_course_in_cache. Retry #{} for this task, exception: {}".format(
+            update_course_in_cache.request.retries,
+            repr(exc)
+        ))
+        raise update_course_in_cache.retry(args=[course_id], exc=exc)

openedx/core/djangoapps/content/block_structure/tests/test_tasks.py

+"""
+Unit tests for the Course Blocks tasks
+"""
+
+from mock import patch
+
+from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase
+
+from ..tasks import update_course_in_cache
+
+
+class UpdateCourseInCacheTaskTest(ModuleStoreTestCase):
+    """
+    Ensures that the update_course_in_cache task runs as expected.
+    """
+    @patch('openedx.core.djangoapps.content.block_structure.tasks.update_course_in_cache.retry')
+    @patch('openedx.core.djangoapps.content.block_structure.api.update_course_in_cache')
+    def test_retry_on_error(self, mock_update, mock_retry):
+        """
+        Ensures that tasks will be retried if IntegrityErrors are encountered.
+        """
+        mock_update.side_effect = Exception("WHAMMY")
+        update_course_in_cache.apply(args=["invalid_course_key raises exception 12345 meow"])
+        self.assertTrue(mock_retry.called)