Page MenuHomePhabricator

Added a configuration option for SMTP timeout.
AbandonedPublic

Authored by yehangjun on Apr 18 2014, 6:04 AM.
Tags
None
Referenced Files
Unknown Object (File)
Wed, Dec 11, 11:56 AM
Unknown Object (File)
Mon, Dec 2, 9:10 AM
Unknown Object (File)
Mon, Dec 2, 4:55 AM
Unknown Object (File)
Tue, Nov 26, 8:47 AM
Unknown Object (File)
Tue, Nov 26, 8:47 AM
Unknown Object (File)
Tue, Nov 26, 8:28 AM
Unknown Object (File)
Oct 20 2024, 1:37 PM
Unknown Object (File)
Oct 16 2024, 8:30 PM
Subscribers

Details

Reviewers
epriestley
Group Reviewers
Blessed Reviewers
Summary

Our smtp server responded slowly sometimes and reached the timeout, but actually
it had sent the email successfully. The mta then retried and sent duplicated
emails. So have an option to let user to set an appropriate timeout.

Test Plan

Tested in our deployed and it worked

Diff Detail

Repository
rP Phabricator
Branch
smtp-timeout
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 1
Build 1: [Placeholder Plan] Wait for 30 Seconds

Event Timeline

yehangjun retitled this revision from to Added a configuration option for SMTP timeout..
yehangjun updated this object.
yehangjun edited the test plan for this revision. (Show Details)
yehangjun added a reviewer: epriestley.

Hmm... let's just increase this to a larger value in all cases. I want to avoid adding configuration unless we really need to (we have a huge number of options already, and adding more tends to make it harder to configure Phabricator), so let's just try using a hard-coded (but larger) value first. I'm comfortable with any number up to, say, 300 seconds. Would that be long enough?

That sounds good to me. I actually changed the default values to 60 seconds in hard-code before I composed this code change. 300 seconds is long enough for my situation. But I'm not sure if setting a larger value has side-effect under other situations.

I personally prefer having a configuration option. The user doesn't have to set it if not necessary, so it doesn't increase user's burden, but it makes the potential problem (if it is) more obvious.

Both are ok to me anyway, that's up to you.

If 60 is OK for you, let's just change it to 60.