Skip to content

Move email handling into a service #3493

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

Merged
merged 2 commits into from
Apr 1, 2018
Merged

Conversation

dstufft
Copy link
Member

@dstufft dstufft commented Apr 1, 2018

This PR moves email handling into a service, this is done to prepare for eventual SES support (which will not go through SMTP, but rather through the SES API). Functionally speaking this doesn't change anything, but codewise a few APIs have changed:

  • warehouse.email:send_email() no longer takes a recipients and a bcc kwarg, instead it takes a single recipient kwarg.
    • By having a single send_email() call per email address, we eliminate issues where an address might be blocked at the server side, causing all addresses to fail to send.
    • Instead of using BCC to handle sending the same email to multiple people, this relies on a simple loop that calls warehouse.email:send_mail() multiple times. This is better because when end users receive the email, it will still have their email in the To: field, instead of no address.
  • The order of subject and body being passed to warehouse.email:send_email() has changed and subject must be passed first.
    • This makes code match the traditional UI for sending email better, where the subject is first, and then the body follows. A minor UX improvement to writing email sending code.

This change does not introduce anything like configurable services for sending email, future PRs will include that, but this moves us closer to being able to swap out the email handling at runtime.

@dstufft
Copy link
Member Author

dstufft commented Apr 1, 2018

Note that this PR conflicts with #3306 and whichever one lands second will have to be updated to handle the other.

@dstufft dstufft merged commit 1ce61ff into pypi:master Apr 1, 2018
@dstufft dstufft deleted the email-service branch April 1, 2018 19:30
nitinprakash96 pushed a commit to nitinprakash96/warehouse that referenced this pull request Apr 3, 2018
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.

1 participant