From a6f4f1762b4482d6b2dee187b5052e48fb8592cc Mon Sep 17 00:00:00 2001 From: Tasawer Nawaz Date: Thu, 7 Dec 2017 15:36:07 +0500 Subject: [PATCH] update logging in award program certiicate task LEARNER-3165 --- .../djangoapps/programs/tasks/v1/tasks.py | 10 ++++- .../programs/tasks/v1/tests/test_tasks.py | 43 +++++++++++++++++-- 2 files changed, 47 insertions(+), 6 deletions(-) diff --git a/openedx/core/djangoapps/programs/tasks/v1/tasks.py b/openedx/core/djangoapps/programs/tasks/v1/tasks.py index 3a60cb2d7a..ff5d8ed673 100644 --- a/openedx/core/djangoapps/programs/tasks/v1/tasks.py +++ b/openedx/core/djangoapps/programs/tasks/v1/tasks.py @@ -194,8 +194,14 @@ def award_program_certificates(self, username): ) except Exception: # pylint: disable=broad-except # 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_uuid, username) - retry = True + warning_message = 'Failed to award certificate for program {uuid} to user {username}.'.format( + uuid=program_uuid, username=username) + + if countdown < MAX_RETRIES: + LOGGER.warning(warning_message) + retry = True + else: + LOGGER.exception("Max retries exceeded. {msg}".format(msg=warning_message)) if retry: # N.B. This logic assumes that this task is idempotent 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 a2dfb2788d..e857d9c3be 100644 --- a/openedx/core/djangoapps/programs/tasks/v1/tests/test_tasks.py +++ b/openedx/core/djangoapps/programs/tasks/v1/tests/test_tasks.py @@ -255,22 +255,57 @@ class AwardProgramCertificatesTestCase(CatalogIntegrationMixin, CredentialsApiCo """ Checks that a single failure to award one of several certificates does not cause the entire task to fail. Also ensures that - successfully awarded certs are logged as INFO and exceptions - that arise are logged also. + successfully awarded certs are logged as INFO and warning is logged + for failed requests if there are retries available. """ mock_get_completed_programs.return_value = [1, 2] mock_get_certified_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: + mock.patch(TASKS_MODULE + '.LOGGER.warning') as mock_warning: tasks.award_program_certificates.delay(self.student.username).get() self.assertEqual(mock_award_program_certificate.call_count, 3) - mock_exception.assert_called_once_with(mock.ANY, 1, self.student.username) + mock_warning.assert_called_once_with( + 'Failed to award certificate for program {uuid} to user {username}.'.format( + uuid=1, + username=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) + @mock.patch(TASKS_MODULE + '.MAX_RETRIES', 0) + def test_retries_exceeded( + self, + mock_get_completed_programs, + mock_get_certified_programs, + mock_award_program_certificate, + ): + """ + Checks that a single failure to award one of several certificates + does not cause the entire task to fail. Also ensures that + successfully awarded certs are logged as INFO and exception is logged + for failed requests if retires are exceeded. + """ + + mock_get_completed_programs.return_value = [1, 2] + mock_get_certified_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(retries=0) + + self.assertEqual(mock_award_program_certificate.call_count, 2) + + mock_exception.assert_called_once_with( + 'Max retries exceeded. Failed to award certificate for program {uuid} to user {username}.'.format( + uuid=1, + username=self.student.username) + ) + mock_info.assert_any_call(mock.ANY, 2, self.student.username) + def test_retry_on_programs_api_errors( self, mock_get_completed_programs,