Re: [Python-Dev] [Python-checkins] cpython: Fix closes Issue11281 - smtplib.STMP gets source_address parameter, which adds
Hi, On 30/07/2011 5.58, senthil.kumaran wrote:
http://hg.python.org/cpython/rev/26839edf3cc1 changeset: 71617:26839edf3cc1 parent: 71613:018e14a46454 user: Senthil Kumaran<senthil@uthcode.com> date: Sat Jul 30 10:56:50 2011 +0800 summary: Fix closes Issue11281 - smtplib.STMP gets source_address parameter, which adds the ability to bind to specific source address on a machine with multiple interfaces. Patch by Paulo Scardine.
files: Doc/library/smtplib.rst | 33 +++++++++++++++++----- Lib/smtplib.py | 40 ++++++++++++++++++--------- Lib/test/mock_socket.py | 3 +- Lib/test/test_smtplib.py | 17 +++++++++++ Misc/NEWS | 4 ++ 5 files changed, 75 insertions(+), 22 deletions(-)
diff --git a/Doc/library/smtplib.rst b/Doc/library/smtplib.rst --- a/Doc/library/smtplib.rst +++ b/Doc/library/smtplib.rst @@ -20,7 +20,7 @@ Protocol) and :rfc:`1869` (SMTP Service Extensions).
-.. class:: SMTP(host='', port=0, local_hostname=None[, timeout]) +.. class:: SMTP(host='', port=0, local_hostname=None[, timeout], source_address=None)
The "[, timeout]" now looks weird there, and it would be better to convert it to ", timeout=..." to match the other args. However I don't know what the value should be, since the real value is socket._GLOBAL_DEFAULT_TIMEOUT (i.e. object()) and I don't think it's a good idea to expose that. Maybe "None" can be used instead?
A :class:`SMTP` instance encapsulates an SMTP connection. It has methods that support a full repertoire of SMTP and ESMTP operations. If the optional @@ -29,7 +29,12 @@ raised if the specified host doesn't respond correctly. The optional *timeout* parameter specifies a timeout in seconds for blocking operations like the connection attempt (if not specified, the global default timeout - setting will be used). + setting will be used). The optional source_address parameter allows to bind to some + specific source address in a machine with multiple network interfaces, + and/or to some specific source tcp port. It takes a 2-tuple (host, port),
I think TCP should be uppercase.
+ for the socket to bind to as its source address before connecting. If + ommited (or if host or port are '' and/or 0 respectively) the OS default
s/ommited/omitted/ and s/''/``''``/
+ behavior will be used.
For normal use, you should only require the initialization/connect, :meth:`sendmail`, and :meth:`quit` methods. An example is included below. @@ -48,8 +53,10 @@ .. versionchanged:: 3.3 Support for the :keyword:`with` statement was added.
+ .. versionadded:: 3.3 + source_address parameter.
I think the convention is to use "versionadded" when the function/method/class/etc has been added, and "versionchanged" for all the changes, including new arguments.
-.. class:: SMTP_SSL(host='', port=0, local_hostname=None, keyfile=None, certfile=None[, timeout], context=None) +.. class:: SMTP_SSL(host='', port=0, local_hostname=None, keyfile=None, certfile=None[, timeout], context=None, source_address=None)
Ditto for "[, timeout]" and the typos/markup below.
A :class:`SMTP_SSL` instance behaves exactly the same as instances of :class:`SMTP`. :class:`SMTP_SSL` should be used for situations where SSL is @@ -62,18 +69,28 @@ keyfile and certfile must be None. The optional *timeout* parameter specifies a timeout in seconds for blocking operations like the connection attempt (if not specified, the global default timeout setting - will be used). + will be used). The optional source_address parameter allows to bind to some + specific source address in a machine with multiple network interfaces, + and/or to some specific source tcp port. It takes a 2-tuple (host, port), + for the socket to bind to as its source address before connecting. If + ommited (or if host or port are '' and/or 0 respectively) the OS default + behavior will be used.
.. versionchanged:: 3.3 *context* was added.
+ .. versionadded:: 3.3 + source_address parameter.
-.. class:: LMTP(host='', port=LMTP_PORT, local_hostname=None) + +.. class:: LMTP(host='', port=LMTP_PORT, local_hostname=None, source_address=None)
The LMTP protocol, which is very similar to ESMTP, is heavily based on the - standard SMTP client. It's common to use Unix sockets for LMTP, so our :meth:`connect` - method must support that as well as a regular host:port server. To specify a - Unix socket, you must use an absolute path for *host*, starting with a '/'. + standard SMTP client. It's common to use Unix sockets for LMTP, so our + :meth:`connect` method must support that as well as a regular host:port + server. The optional parameters local_hostname and source_address has the
s/has/have/? Also I prefer 'arguments' rather than 'parameters', the smtplib doc uses both, but 'arguments' seems to be used more.
+ same meaning as that of SMTP client.To specify a Unix socket, you must use
Missing space after the '.' (there should be two spaces, but here a single space is used consistently so it's fine).
+ an absolute path for *host*, starting with a '/'.
Authentication is supported, using the regular SMTP mechanism. When using a Unix socket, LMTP generally don't support or require any authentication, but your diff --git a/Lib/smtplib.py b/Lib/smtplib.py --- a/Lib/smtplib.py +++ b/Lib/smtplib.py @@ -215,7 +215,8 @@ [...]
Best Regards, Ezio Melotti
On Sat, Jul 30, 2011 at 11:11:08PM +0300, Ezio Melotti wrote:
-.. class:: SMTP(host='', port=0, local_hostname=None[, timeout]) +.. class:: SMTP(host='', port=0, local_hostname=None[, timeout], source_address=None)
The "[, timeout]" now looks weird there, and it would be better to convert it to ", timeout=..." to match the other args. However I don't know what the value should be, since the real value is socket._GLOBAL_DEFAULT_TIMEOUT (i.e. object()) and I don't think it's a good idea to expose that. Maybe "None" can be used instead?
I found that [, timeout] bit odd too. But is not mentioning that as timeout=None when it is timeout=socket._GLOBAL_DEFAULT_TIME technically inaccurate? FWIW, I see similar style (...,[,timeout], kw=val) adopted elsewhere in the library docs too. urllib, httplib, nntplib etc. Though it does not look okay, it is better than giving inaccurate information. While ftplib and poplib, has them as timeout=None, while they default to socket._GLOBAL_DEFAULT_TIMEOUT object. If we decide upon something, it can be made consistent. So, the question is, when the timeout argument refers to socket._GLOBAL_DEFAULT_TIME, how should we write the docs. 1. def somesocketmethod(arg1,arg2, timeout=socket._GLOBAL_DEFAULT_TIMEOUT, ...) - I don't see anything is wrong with this. 2. def somesocketmethod(arg1,arg2, timeout=None, ...) - And explain that it actually points to a socket default timeout object, which is odd and can lead to user errors. 3. def somesocketmethod(arg1,arg2[,timeout], kwarg=value) - That's how it is currently explained at some places. If this style is okay, we can change the places were it refers to None to be above. Thanks for your review comments, I have address the remaining ones. -- Senthil
On 31 Jul 2011, at 02:26, Senthil Kumaran wrote:
On Sat, Jul 30, 2011 at 11:11:08PM +0300, Ezio Melotti wrote:
-.. class:: SMTP(host='', port=0, local_hostname=None[, timeout]) +.. class:: SMTP(host='', port=0, local_hostname=None[, timeout], source_address=None)
The "[, timeout]" now looks weird there, and it would be better to convert it to ", timeout=..." to match the other args. However I don't know what the value should be, since the real value is socket._GLOBAL_DEFAULT_TIMEOUT (i.e. object()) and I don't think it's a good idea to expose that. Maybe "None" can be used instead?
I found that [, timeout] bit odd too. But is not mentioning that as timeout=None when it is timeout=socket._GLOBAL_DEFAULT_TIME technically inaccurate?
It does mean that users will expect to be able to call with an explicit timeout=None and get the default behaviour. Just use the numeric value of the global timeout perhaps? MIchael Foord
FWIW, I see similar style (...,[,timeout], kw=val) adopted elsewhere in the library docs too. urllib, httplib, nntplib etc. Though it does not look okay, it is better than giving inaccurate information.
While ftplib and poplib, has them as timeout=None, while they default to socket._GLOBAL_DEFAULT_TIMEOUT object.
If we decide upon something, it can be made consistent. So, the question is, when the timeout argument refers to socket._GLOBAL_DEFAULT_TIME, how should we write the docs.
1. def somesocketmethod(arg1,arg2, timeout=socket._GLOBAL_DEFAULT_TIMEOUT, ...)
- I don't see anything is wrong with this.
2. def somesocketmethod(arg1,arg2, timeout=None, ...)
- And explain that it actually points to a socket default timeout object, which is odd and can lead to user errors.
3. def somesocketmethod(arg1,arg2[,timeout], kwarg=value)
- That's how it is currently explained at some places. If this style is okay, we can change the places were it refers to None to be above.
Thanks for your review comments, I have address the remaining ones.
-- http://www.voidspace.org.uk/ May you do good and not evil May you find forgiveness for yourself and forgive others May you share freely, never taking more than you give. -- the sqlite blessing http://www.sqlite.org/different.html
On Sun, 31 Jul 2011 17:17:00 +0100 Michael Foord <fuzzyman@voidspace.org.uk> wrote:
I found that [, timeout] bit odd too. But is not mentioning that as timeout=None when it is timeout=socket._GLOBAL_DEFAULT_TIME technically inaccurate?
It does mean that users will expect to be able to call with an explicit timeout=None and get the default behaviour. Just use the numeric value of the global timeout perhaps?
The global timeout is controllable at runtime through socket.setdefaulttimeout(). That's the whole point of using an opaque sentinel. Regards Antoine.
participants (4)
-
Antoine Pitrou -
Ezio Melotti -
Michael Foord -
Senthil Kumaran