Code review comment for lp:~futatuki/mailman/2.1-add-smtp-timeout

Thank you for your consideration for this issue.

> But there is an issue in how socket.timeout is handled in OutgoingRunner.
> Currently, OutgoingRunner treats all socket exceptions as a failure to
> connect. I think I'd want to catch socket.timeout in SMTPDirect, log the fact
> and then raise SomeRecipientsFailed instead.

To accomplish it, there seems to be some different ways.
(1) inspect SMTPDirect exception to find what causes it.
(2) overriding methods of smtplib.SMTP catches socket.error (send() and getreply(), on current implementation of smtplib module (of CPython 2.x). socket.timeout can be caused in connect(), but it is not caught in connect()).
(3) write our own implementation for SMTP, instead of smtplib
(4) other...

(1) and (2) depend on undocumented part of smtplib (of current implementation in CPython 2) as Python standard library, though.

What kind of way to take do you think? (I'd like to help you for this issue if I can do something for it.)

> Also I think it would be appropriate to set the default other than None. Maybe
> something like one minute or five minutes, but I'm not sure what a good value
> would be.

I agree with None is not appropriate for default, but I also have no idea what is good. It is why I choose None for default value :-)
I think one minute is short even if the MTA trust QutgoingRunner, if checking message body, etc, but of course, this is only my personal opinion.

« Back to merge proposal