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

Timeouts for post_to_esp() #80

Closed
nuschk opened this issue Jan 17, 2018 · 3 comments
Closed

Timeouts for post_to_esp() #80

nuschk opened this issue Jan 17, 2018 · 3 comments

Comments

@nuschk
Copy link

nuschk commented Jan 17, 2018

We recently had networking troubles which lead to certain (Sendgrid) SSL handshakes stalling forever. This lead all our mail sending workers to eventually stall and lock up all background tasks.

Usually, for other HTTPS requests, we add a (generous) timeout (see http://docs.python-requests.org/en/master/user/quickstart/#timeouts). The timeout is chosen such that it only triggers in those rare, extreme circumstances. We mostly use 30 seconds, but Stripe, as another example, uses even 80 seconds (https://github.com/stripe/stripe-python/blob/f948b8b95b6df5b57c7444a05d6c83c8c5e6a0ac/stripe/http_client.py#L97).

I think anymail should have a default timeout, too. Doesn't even have to be configurable, and the exception could be passed straight to user code.

@medmunds
Copy link
Contributor

Oh, good call. It should be straightforward to set a timeout for all the requests-based backends (currently all but SparkPost), and easy enough to make this an Anymail setting.

I'm leaning toward a 30 second default timeout. Frankly, this feels really long (though I don't have a good sense for every ESP's historical performance). Thoughts?

@nuschk
Copy link
Author

nuschk commented Jan 18, 2018

I completely agree on a long timeout, which should never be reached by accident. 30s seems completely fine to me and would certainly work for us.

I could imagine people sending recipient lists with several tens of thousands of entries to the ESP. If your line is bad or congested, this may take some time. OTOH, this seems like a very special use case, and if users run into that, it should be very straightforward to diagnose and fix.

So yeah, I'm fine with it. :-)

And thanks for that awesome package and the quick migration away from mandrill! That really made my day back then.

@medmunds
Copy link
Contributor

medmunds commented Feb 2, 2018

Released in v1.3

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

No branches or pull requests

2 participants