Retry the task to grant Programs credentials (which is idempotent) if any constituent program fails

openedx/core/djangoapps/programs/tasks/v1/tasks.py

@@ -200,6 +200,8 @@ def award_program_certificates(self, username):
     # For each completed program for which the student doesn't already have a
     # certificate, award one now.
     #
+    # This logic is important, because we will retry the whole task if awarding any particular program cert fails.
+    #
     # N.B. the list is sorted to facilitate deterministic ordering, e.g. for tests.
     new_program_ids = sorted(list(set(program_ids) - set(existing_program_ids)))
     if new_program_ids:
@@ -213,10 +215,17 @@ def award_program_certificates(self, username):
             # Retry because a misconfiguration could be fixed
             raise self.retry(exc=exc, countdown=countdown, max_retries=config.max_retries)
 
+        retry = False
         for program_id in new_program_ids:
             try:
                 award_program_certificate(credentials_client, username, program_id)
                 LOGGER.info('Awarded certificate for program %s to user %s', program_id, username)
             except Exception:  # pylint: disable=broad-except
-                # keep trying to award other certs.
+                # keep trying to award other certs, but retry the whole task to fix any missing entries
                 LOGGER.exception('Failed to award certificate for program %s to user %s', program_id, username)
+                retry = True
+
+        if retry:
+            # N.B. This logic assumes that this task is idempotent
+            LOGGER.info('Retrying task to award failed certificates to user %s', username)
+            raise self.retry(countdown=countdown, max_retries=config.max_retries)

openedx/core/djangoapps/programs/tasks/v1/tests/test_tasks.py

@@ -331,9 +331,10 @@ class AwardProgramCertificatesTestCase(TestCase, ProgramsApiConfigMixin, Credent
 
         """
         def side_effect(*_a):  # pylint: disable=missing-docstring
-            exc = side_effects.pop(0)
-            if exc:
-                raise exc
+            if side_effects:
+                exc = side_effects.pop(0)
+                if exc:
+                    raise exc
             return mock.DEFAULT
         return side_effect
 
@@ -351,16 +352,17 @@ class AwardProgramCertificatesTestCase(TestCase, ProgramsApiConfigMixin, Credent
         that arise are logged also.
         """
         mock_get_completed_programs.return_value = [1, 2]
-        mock_get_awarded_certificate_programs.return_value = []
+        mock_get_awarded_certificate_programs.side_effect = [[], [2]]
         mock_award_program_certificate.side_effect = self._make_side_effect([Exception('boom'), None])
 
         with mock.patch(TASKS_MODULE + '.LOGGER.info') as mock_info, \
                 mock.patch(TASKS_MODULE + '.LOGGER.exception') as mock_exception:
             tasks.award_program_certificates.delay(self.student.username).get()
 
-        self.assertEqual(mock_award_program_certificate.call_count, 2)
+        self.assertEqual(mock_award_program_certificate.call_count, 3)
         mock_exception.assert_called_once_with(mock.ANY, 1, self.student.username)
-        mock_info.assert_called_with(mock.ANY, 2, self.student.username)
+        mock_info.assert_any_call(mock.ANY, 1, self.student.username)
+        mock_info.assert_any_call(mock.ANY, 2, self.student.username)
 
     def test_retry_on_certificates_api_errors(
             self,