Skip to content
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

raise and handle exception on temporary mail failure #1210

Merged
merged 1 commit into from Jan 5, 2017

Conversation

struan
Copy link
Collaborator

@struan struan commented Jan 5, 2017

If we get a temporary failure message then ignore it as we only need to
do something if the failure is permanent.

@struan struan requested a review from mhl January 5, 2017 15:19
@coveralls
Copy link

coveralls commented Jan 5, 2017

Coverage Status

Coverage decreased (-0.009%) to 98.154% when pulling b2db603 on delayed-delivery-warnings into e7b18bc on alpaca-rebased.

Copy link
Collaborator

@mhl mhl left a comment

Choose a reason for hiding this comment

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

I've suggested a small change, otherwise this looks fine.

@@ -152,7 +152,9 @@ def instanciate_answer(self, lines):
msg = email.message_from_string(msgtxt)
temporary, permanent = all_failures(msg)

if temporary or permanent:
if temporary:
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm not sure if in practice this happens with Exim, but flufl might return both email addresses with temporary failures and permanent failures, and if there are any in permanent it should still register a bounce. So maybe this condition should be if temporary and not permanent. (Or switch the cases round so it's if permanent ... elif temporary ... else ... end.)

@coveralls
Copy link

coveralls commented Jan 5, 2017

Coverage Status

Coverage decreased (-0.006%) to 98.156% when pulling bf90034 on delayed-delivery-warnings into e7b18bc on alpaca-rebased.

If we get a temporary failure message then ignore it as we only need to
do something if the failure is permanent.
@struan struan merged commit b2e64e9 into alpaca-rebased Jan 5, 2017
@struan struan removed the in progress label Jan 5, 2017
@coveralls
Copy link

coveralls commented Jan 5, 2017

Coverage Status

Coverage decreased (-0.009%) to 98.156% when pulling b2e64e9 on delayed-delivery-warnings into 225a708 on alpaca-rebased.

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.

None yet

3 participants