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

2011-07-31 Thread Michael Foord
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






___
Python-Dev mailing list
Python-Dev@python.org
http://mail.python.org/mailman/listinfo/python-dev
Unsubscribe: 
http://mail.python.org/mailman/options/python-dev/archive%40mail-archive.com


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

2011-07-31 Thread Antoine Pitrou
On Sun, 31 Jul 2011 17:17:00 +0100
Michael Foord fuzzy...@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.


___
Python-Dev mailing list
Python-Dev@python.org
http://mail.python.org/mailman/listinfo/python-dev
Unsubscribe: 
http://mail.python.org/mailman/options/python-dev/archive%40mail-archive.com


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

2011-07-30 Thread 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 Kumaransent...@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 

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

2011-07-30 Thread Senthil Kumaran
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
___
Python-Dev mailing list
Python-Dev@python.org
http://mail.python.org/mailman/listinfo/python-dev
Unsubscribe: 
http://mail.python.org/mailman/options/python-dev/archive%40mail-archive.com