Skip to content

Fix retries logic in award program certificate task - #16900

Merged
tasawernawaz merged 1 commit into
masterfrom
tasawer/learner-3165-fix-retries-logic-in-award-program-certificate
Dec 14, 2017
Merged

Fix retries logic in award program certificate task#16900
tasawernawaz merged 1 commit into
masterfrom
tasawer/learner-3165-fix-retries-logic-in-award-program-certificate

Conversation

@tasawernawaz

Copy link
Copy Markdown
Contributor

LEARNER-3165

Previous PR: #16822

@bderusha Please review.

@bderusha bderusha left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code looks good. One addition requested

retry = True
else:
LOGGER.exception("Max retries exceeded. {msg}".format(msg=warning_message))
LOGGER.warning('Failed to award certificate for program {uuid} to user {username}.'.format(

@bderusha bderusha Dec 13, 2017

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that this looks good. Can we also add an additional warning if this is the last attempt

if self.request.retries == MAX_RETRIES:
    LOGGER.exception('Final attempt to award certificate for program %s to user %s failed', program_uuid, username)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

::EDIT::

Actually I think this may not be necessary as you mentioned in the other PR. But we should update our alerting to capture whatever error message is output when MaxRetriesExceededError is thrown

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bderusha Got it, thanks.

@tasawernawaz
tasawernawaz merged commit 0a0a17c into master Dec 14, 2017
@tasawernawaz
tasawernawaz deleted the tasawer/learner-3165-fix-retries-logic-in-award-program-certificate branch December 14, 2017 07:15
@edx-pipeline-bot

Copy link
Copy Markdown
Contributor

EdX Release Notice: This PR has been deployed to the staging environment in preparation for a release to production on Tuesday, December 19, 2017.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants