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

Add support for Mailtrap #406

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from
Draft

Add support for Mailtrap #406

wants to merge 2 commits into from

Conversation

cahna
Copy link

@cahna cahna commented Nov 10, 2024

Hi, and thanks for creating such a great library! I would love to have Mailtrap support, and I figured it would be helpful if I started on that. This is very much a work-in-progress, as I'm still trying to understand how to handle all of the features that Anymail provides, and whether those features are supported by Mailtrap. I have already caught at least one category of features that Mailtrap does not support: inbound emails and reply-to.

If anyone would be willing to provide some feedback on what I have, and any guidance on how to continue, I would appreciate it, and I will be sure to add tests and documentation.

I am also having trouble getting local builds and tests to work, or I would have started on adding tests. If someone could point me in the direction of minimal steps to get tests working on Ubuntu, I would appreciate it. (The contributing docs don't seem to work for me).

Cheers

test_api_url += "/"
self.test_api_url = test_api_url

bulk_api_url = get_anymail_setting(
Copy link
Author

Choose a reason for hiding this comment

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

I haven't quite determined how Backends should handle bulk emails, but it also does not appear that Mailtrap's APIs are any different for their "Email sending" vs "Bulk email" APIs https://api-docs.mailtrap.io/docs/mailtrap-api-docs/.

Maybe this self.bulk_api_url is unnecessary, and it should be up to the user to configure which API url to use via self.api_url?

)

# Not the best status reporting. Mailtrap only says that the order of message-ids will be in this order (but JSON is unordered?)
recipient_status_order = [*message.to, *message.cc, *message.bcc]
Copy link
Author

Choose a reason for hiding this comment

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

Mailtrap's status reporting is pretty poor, and I'm not even sure if this will work consistently, given that it is depending on ORDERED JSON responses 🤦

@cahna cahna force-pushed the mailtrap branch 4 times, most recently from c3b2c2d to 8c44826 Compare November 10, 2024 23:49
@medmunds
Copy link
Contributor

Thanks for this! It looks pretty good at first glance; I'll try to take a closer look later this week.

Mailtrap does not support: inbound emails and reply-to.

Missing inbound support is not unusual; it's fine to just ignore it.

Reply-to is a little surprising. Some ESPs require handling this as an extra header. (I haven't had a chance to look at Mailtrap's docs.)

I am also having trouble getting local builds and tests to work

What's going wrong? (In the contributing docs, I notice the "test a representative combination of Python and Django versions" command is outdated—the current version should be tox -e lint,django51-py312-all,django40-py38-all,docs. But other than that I'd expect it to work. I'm usually on macOS, but the GitHub tests all run on Ubuntu.)

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

Successfully merging this pull request may close these issues.

2 participants