fix: replacing exceptions #31827
fix: replacing exceptions #31827
Conversation
awais786
force-pushed
the
boto-ses.exception-to-boto3
branch
2 times, most recently
from
89ad5c7
to
f0339ad
Compare
| @@ -444,18 +444,6 @@ def _test_immediate_failure(self, exception): | |||
| def test_failure_on_unhandled_smtp(self): | |||
| self._test_immediate_failure(SMTPAuthenticationError(403, "That password doesn't work!")) | |||
|
|
|||
| def test_failure_on_ses_quota_exceeded(self): | |||
There was a problem hiding this comment.
Things already working without these exceptions.
UsamaSadiq
force-pushed
the
boto-ses.exception-to-boto3
branch
from
cadb05a
to
5fbf98b
Compare
lms/djangoapps/bulk_email/tasks.py
Outdated
| ) | ||
| subtask_status.increment(failed=1) | ||
| if exc.response['Error']['Code'] in [ | ||
| 'InvalidParameterValue', 'MessageRejected', 'MailFromDomainNotVerified' |
There was a problem hiding this comment.
It seems like InvalidParameterValue may not be an exception that SES recognizes/throws? I'm looking at the list here: https://botocore.amazonaws.com/v1/documentation/api/latest/reference/services/ses.html#client-exceptions. I see MessageRejected and MailFromDomainNotVerified, but not InvalidParameterValue. Should this be included?
| SESAddressNotVerifiedError, # Raised when a "Reply-To" address has not been validated in SES yet. | ||
| SESIdentityNotVerifiedError, # Raised when an identity has not been verified in SES yet. | ||
| SESDomainNotConfirmedError, # Raised when domain ownership is not confirmed for DKIM. | ||
| SESDailyQuotaExceededError, # 24-hour allotment of outbound email has been exceeded. |
There was a problem hiding this comment.
I see that we removed a number of these failure exceptions, are there any introduced in botocore/boto3 that we can/should take advantage of?
I was looking to see if the ones we removed mapped to any of the new/currently supported exceptions (here: https://botocore.amazonaws.com/v1/documentation/api/latest/reference/services/ses.html#client-exceptions) and I saw items such as AccountSendingPausedException, MailFromDomainNotVerifiedException, LimitExceededException that seemed similar.
There was a problem hiding this comment.
Can you continue working on this? We have very limited domain knowledge here to replace all those exceptions.
|
@justinhynes any plan for this PR ?. Its blocking us to upgrade |
|
Since we are catching clienterror at multiple locations so in case of no matching error code raise the exception again. |
|
@justinhynes Kindly review this PR. Also I have a question I did't find any exception log on splunk for last couple of weeks. |
This reverts commit fcb4c4d.
|
EdX Release Notice: This PR has been deployed to the staging environment in preparation for a release to production. |
|
EdX Release Notice: This PR has been deployed to the production environment. |
For details check this issue
**Removed boto related exceptions. **
edx-appis using latest version ofdjango-seswhich usesboto3for mail sending. So apparently exceptions being in use are not working, these were implemented forbotoand inboto3they have changed all exceptions.