Operation-level timeout

[wombatonfire]

Is your feature request related to a problem? Please describe.

It may be reasonable to have different timeouts for different operations during the SMTP session. Specifically, you may want connect() to fail fast when provided server details were clearly wrong, but give send_message() some more time to wait for a response from the server.

Based on the SMTP class documentation you may think that there is a "global" client timeout (timeout= in SMTP.__init__()), which can be overridden for a specific operation (method) using a timeout keyword argument. The problem is that, when you do it, you do not simply override the timeout for this operation, but you change the "global" client timeout. The new timeout value is saved as self.timeout and will be used for all subsequent operations.

This is, probably, not what you would expect, and requires an explicit reset of the timeout in the user code.

Describe the solution you'd like

I think, when timeout is overridden on the operation level, it should not be processed in _update_settings_from_kwargs() along with other keyword arguments and it should not overwrite the client-level value. When this specific operation is completed, all subsequent operations should continue using the client-level timeout, and not the operation-level override.

Describe alternatives you've considered

The alternative is to explicitly reset the client-level timeout to the value used before the override was specified. This is what I do now.

[cole]

Thanks for reporting!

Current expected behaviour is that two timeout kwargs set the instance value– __init__ and connect. Other methods should not persist the timeout. This might not be documented correctly.

I agree this doesn't work well for the case where the connect timeout should be different than the default.

I'll have a look but I think the suggested change is reasonable, probably I can proceed with that 👍

[wombatonfire]

Hi Cole, thanks for the great lib!

I'm happy to contribute and implement this change. I haven't checked other methods, only connect. Let me see how they handle timeout without persisting it, and I'll create a pull request.

[wombatonfire]

Ok, so SMTP.timeout is updated by _update_settings_from_kwargs() in connect() and starttls(). I changed those methods to handle passed timeout value similar to how execute_command() does.

This required changing the signature of _create_connection() and adding the timeout argument, but I suppose it shouldn't be an issue, since it's a private method.

The timeout argument was also removed from _update_settings_from_kwargs() as it's not supposed to be updated there anymore.

If this makes sense, I suppose some tests should be added?

[wombatonfire]

Didn't realize the pull request was created in the wrong repository/branch.