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

Revision history for this message
Mark Sapiro (msapiro) wrote :

Thank you for this contribution. I'm still considering how to handle this. I'd like to make Python 2.7 or at least Python 2.6 a minimum requirement, but that's not really an issue as your code handles older versions.

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.

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.

« Back to merge proposal