diff --git a/openedx/core/djangoapps/programs/tasks/v1/tasks.py b/openedx/core/djangoapps/programs/tasks/v1/tasks.py index b3888f5761..99462b92e8 100644 --- a/openedx/core/djangoapps/programs/tasks/v1/tasks.py +++ b/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) diff --git a/openedx/core/djangoapps/programs/tasks/v1/tests/test_tasks.py b/openedx/core/djangoapps/programs/tasks/v1/tests/test_tasks.py index 487c5a2079..d217ddf489 100644 --- a/openedx/core/djangoapps/programs/tasks/v1/tests/test_tasks.py +++ b/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,