[Python-Dev] [Python-checkins] cpython: Fix closes Issue11281 - smtplib.STMP gets source_address parameter, which adds

[Ezio Melotti]

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 at 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