-
Notifications
You must be signed in to change notification settings - Fork 3.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix: replacing exceptions #31827
Merged
Merged
fix: replacing exceptions #31827
Changes from 10 commits
Commits
Show all changes
11 commits
Select commit
Hold shift + click to select a range
5fbf98b
fix: fixing test.
awais786 ced8f7d
Merge branch 'master' into boto-ses.exception-to-boto3
awais786 238ae15
fix: updating exceptions.
awais786 de65cbb
fix: updating exceptions.
awais786 eb12b77
fix: updating exceptions.
awais786 85cf18a
Merge branch 'master' into boto-ses.exception-to-boto3
awais786 031cff4
Merge branch 'master' into boto-ses.exception-to-boto3
awais786 ab9b4d2
Merge branch 'master' into boto-ses.exception-to-boto3
awais786 f2ab900
Merge branch 'master' into boto-ses.exception-to-boto3
awais786 ef2d2f1
Merge branch 'master' into boto-ses.exception-to-boto3
awais786 01e5af4
Merge branch 'master' into boto-ses.exception-to-boto3
awais786 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -7,27 +7,23 @@ | |
""" | ||
|
||
|
||
from datetime import datetime | ||
from dateutil.relativedelta import relativedelta | ||
import json # lint-amnesty, pylint: disable=wrong-import-order | ||
from datetime import datetime | ||
from itertools import chain, cycle, repeat # lint-amnesty, pylint: disable=wrong-import-order | ||
from smtplib import SMTPAuthenticationError, SMTPConnectError, SMTPDataError, SMTPServerDisconnected, SMTPSenderRefused # lint-amnesty, pylint: disable=wrong-import-order | ||
from smtplib import ( # lint-amnesty, pylint: disable=wrong-import-order | ||
SMTPAuthenticationError, | ||
SMTPConnectError, | ||
SMTPDataError, | ||
SMTPSenderRefused, | ||
SMTPServerDisconnected | ||
) | ||
from unittest.mock import Mock, patch # lint-amnesty, pylint: disable=wrong-import-order | ||
from uuid import uuid4 # lint-amnesty, pylint: disable=wrong-import-order | ||
|
||
import pytest | ||
from boto.exception import AWSConnectionError | ||
from boto.ses.exceptions import ( | ||
SESAddressBlacklistedError, | ||
SESAddressNotVerifiedError, | ||
SESDailyQuotaExceededError, | ||
SESDomainEndsWithDotError, | ||
SESDomainNotConfirmedError, | ||
SESIdentityNotVerifiedError, | ||
SESIllegalAddressError, | ||
SESLocalAddressCharacterError, | ||
SESMaxSendingRateExceededError | ||
) | ||
from botocore.exceptions import ClientError, EndpointConnectionError | ||
from celery.states import FAILURE, SUCCESS | ||
from dateutil.relativedelta import relativedelta | ||
from django.conf import settings | ||
from django.core.management import call_command | ||
from django.test.utils import override_settings | ||
|
@@ -268,19 +264,22 @@ def test_smtp_blacklisted_user(self): | |
|
||
def test_ses_blacklisted_user(self): | ||
# Test that celery handles permanent SMTPDataErrors by failing and not retrying. | ||
self._test_email_address_failures(SESAddressBlacklistedError(554, "Email address is blacklisted")) | ||
|
||
def test_ses_illegal_address(self): | ||
# Test that celery handles permanent SMTPDataErrors by failing and not retrying. | ||
self._test_email_address_failures(SESIllegalAddressError(554, "Email address is illegal")) | ||
operation_name = '' | ||
parsed_response = {'Error': {'Code': 'MessageRejected', 'Message': 'Error Uploading'}} | ||
self._test_email_address_failures(ClientError(parsed_response, operation_name)) | ||
|
||
def test_ses_local_address_character_error(self): | ||
def test_ses_illegal_address(self): | ||
# Test that celery handles permanent SMTPDataErrors by failing and not retrying. | ||
self._test_email_address_failures(SESLocalAddressCharacterError(554, "Email address contains a bad character")) | ||
operation_name = '' | ||
parsed_response = {'Error': {'Code': 'MailFromDomainNotVerifiedException', 'Message': 'Error Uploading'}} | ||
self._test_email_address_failures(ClientError(parsed_response, operation_name)) | ||
|
||
def test_ses_domain_ends_with_dot(self): | ||
# Test that celery handles permanent SMTPDataErrors by failing and not retrying. | ||
self._test_email_address_failures(SESDomainEndsWithDotError(554, "Email address ends with a dot")) | ||
operation_name = '' | ||
parsed_response = {'Error': {'Code': 'MailFromDomainNotVerifiedException', 'Message': 'invalid domain'}} | ||
self._test_email_address_failures(ClientError(parsed_response, operation_name)) | ||
|
||
def test_bulk_email_skip_with_non_ascii_emails(self): | ||
""" | ||
|
@@ -367,12 +366,12 @@ def test_max_retry_after_smtp_connect_error(self): | |
|
||
def test_retry_after_aws_connect_error(self): | ||
self._test_retry_after_limited_retry_error( | ||
AWSConnectionError("Unable to provide secure connection through proxy") | ||
EndpointConnectionError(endpoint_url="Could not connect to the endpoint URL:") | ||
) | ||
|
||
def test_max_retry_after_aws_connect_error(self): | ||
self._test_max_retry_limit_causes_failure( | ||
AWSConnectionError("Unable to provide secure connection through proxy") | ||
EndpointConnectionError(endpoint_url="Could not connect to the endpoint URL:") | ||
) | ||
|
||
def test_retry_after_general_error(self): | ||
|
@@ -416,11 +415,6 @@ def test_retry_after_smtp_sender_refused_error(self): | |
SMTPSenderRefused(421, "Throttling: Sending rate exceeded", self.instructor.email) | ||
) | ||
|
||
def test_retry_after_ses_throttling_error(self): | ||
self._test_retry_after_unlimited_retry_error( | ||
SESMaxSendingRateExceededError(455, "Throttling: Sending rate exceeded") | ||
) | ||
|
||
def _test_immediate_failure(self, exception): | ||
"""Test that celery can hit a maximum number of retries.""" | ||
# Doesn't really matter how many recipients, since we expect | ||
|
@@ -444,18 +438,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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Things already working without these exceptions. |
||
self._test_immediate_failure(SESDailyQuotaExceededError(403, "You're done for the day!")) | ||
|
||
def test_failure_on_ses_address_not_verified(self): | ||
self._test_immediate_failure(SESAddressNotVerifiedError(403, "Who *are* you?")) | ||
|
||
def test_failure_on_ses_identity_not_verified(self): | ||
self._test_immediate_failure(SESIdentityNotVerifiedError(403, "May I please see an ID!")) | ||
|
||
def test_failure_on_ses_domain_not_confirmed(self): | ||
self._test_immediate_failure(SESDomainNotConfirmedError(403, "You're out of bounds!")) | ||
|
||
def test_bulk_emails_with_unicode_course_image_name(self): | ||
# Test bulk email with unicode characters in course image name | ||
course_image = '在淡水測試.jpg' | ||
|
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you continue working on this? We have very limited domain knowledge here to replace all those exceptions.