| msg129035 - (view) |
Author: Paulo Scardine (paulos) |
Date: 2011-02-22 04:10 |
| In smtplib there is now way to bind to an specific source address on a machine with multiple interfaces; also, there no way to control the source port for the connection. Since 2.7, socket.create_connection accepts a source_address parameter, a (host, port) tuple for the socket to bind to as its source address before connecting. If host or port are '' or 0 respectively the OS default behavior will be used. I would like to add source_ip and source_port parameters to smtplib.SMTP, default to '' and 0 respectively. It is a small change with no impact over existing code. Should I submit a patch? |
|
|
| msg129059 - (view) |
Author: R. David Murray (r.david.murray) *  |
Date: 2011-02-22 12:19 |
| This sounds like a reasonable feature request. If you would like to propose a patch against trunk (py3k, what will become 3.3), I will take a look at it. |
|
|
| msg129085 - (view) |
Author: Giampaolo Rodola' (giampaolo.rodola) * |
Date: 2011-02-22 15:36 |
| +1, this has been done for other modules such as ftplib as well and probably could be done for others such as httplib and poplib. |
|
|
| msg129092 - (view) |
Author: Giampaolo Rodola' (giampaolo.rodola) * |
Date: 2011-02-22 16:17 |
| > I would like to add source_ip and source_port parameters to > smtplib.SMTP, default to '' and 0 respectively. It would be better to provide a unique source_address parameter defaulting to None, for consistency with socket.create_connection() expecting a (addr, port) tuple. |
|
|
| msg129098 - (view) |
Author: Paulo Scardine (paulos) |
Date: 2011-02-22 17:12 |
| Seems like the signature of Lib.test.mock_socket.create_connection does not match socket.create_connection since 2.7. I need help with the docs; the last argument is positional and optional, using the `[ ]' notation. What is the notation for the new keyword arguments? |
|
|
| msg129100 - (view) |
Author: Giampaolo Rodola' (giampaolo.rodola) * |
Date: 2011-02-22 17:16 |
| > Seems like the signature of Lib.test.mock_socket.create_connection does > not match socket.create_connection since 2.7. You can add a fake parameter which does nothing. > What is the notation for the new keyword arguments? fun(a, b, c=None, d=None) |
|
|
| msg129101 - (view) |
Author: Paulo Scardine (paulos) |
Date: 2011-02-22 17:18 |
| Also, it is my first patch and I have no clue about what I'm doing, I don't expect to get it right in the first try - please point any mistakes. |
|
|
| msg129103 - (view) |
Author: Paulo Scardine (paulos) |
Date: 2011-02-22 17:22 |
| @Giampaolo: should I change the text in Doc/library/smtplib.rst or it is infered from the docstrings? |
|
|
| msg129106 - (view) |
Author: Giampaolo Rodola' (giampaolo.rodola) * |
Date: 2011-02-22 17:37 |
| Yes, you must update Doc/library/smtplib.rst and write tests. If you're not the one who's gonna commit the patch you can ignore Doc/whatsnew/3.3.srt and Misc/NEWS changes. Updating the docstring is usually optional but I see smtplib module is already well documented so I'd update docstrings as well. As for how to write a patch see: http://www.python.org/dev/faq/#patches ...or search in the bug tracker for committed patches, such as: http://bugs.python.org/issue8807 http://svn.python.org/view?view=revision&revision=84144 |
|
|
| msg129108 - (view) |
Author: Paulo Scardine (paulos) |
Date: 2011-02-22 18:33 |
| My first idea was to make the argument a tuple to match socket.create_connection(), but SMTP uses host and port arguments, for consistency it's better havin separate source_ip and source_port arguments. As a side effect, it makes easier to specify only source_port. The submited patch includes tests and updated docstrings; At the smtplib.rst, for example: SMTP(host='', port=0, local_hostname=None[, timeout]) Would be ok to change it like: SMTP(host='', port=0, local_hostname=None[, timeout], source_ip='', source_port=0) |
|
|
| msg129112 - (view) |
Author: Giampaolo Rodola' (giampaolo.rodola) * |
Date: 2011-02-22 19:12 |
| We already have 3 places where a tuple is used: socket.socket.bind socket.create_connection http.client.HTTPConnection Changing this notation in smtplib for such a rarely used feature is not worth the effort, imo. Also, I would not add two new SMTP attributes. Passing a tuple to socket.create_connection() is just fine. If you want to know the source address afterwards you can use socket.getsockname(). PS - I modified smtplib.py in meantime. Please update your local copy. |
|
|
| msg129150 - (view) |
Author: Paulo Scardine (paulos) |
Date: 2011-02-23 01:32 |
| Ok, I changed to a single parameter matching socket.create_connection(). I made my best to update tests and docs, but I don't have a clue if it is right. |
|
|
| msg129152 - (view) |
Author: Giampaolo Rodola' (giampaolo.rodola) * |
Date: 2011-02-23 01:49 |
| Follow my comments: > + source_address=('', 0)): Make that default to None instead and pass it as-is to socket.create_connection(). > + self.source_address = source_address There's no need to store the source address as an instance attribute. Just pass it as-is to socket.create_connection() in __init__ and connect methods and then get rid of it. I think source_address no longer makes sense in UNIX sockets / LMTP class. Or at least, this is what I get if I try to bind() a UNIX socket: >>> import socket >>> sock = socket.socket(socket.AF_UNIX, socket.SOCK_STREAM) >>> sock.bind(("", 0)) Traceback (most recent call last): File "", line 1, in File "", line 1, in bind TypeError: argument must be string or read-only character buffer, not tuple >>> |
|
|
| msg129153 - (view) |
Author: Paulo Scardine (paulos) |
Date: 2011-02-23 02:58 |
| > There's no need to store the source address as an instance attribute. > Just pass it as-is to socket.create_connection() in __init__ and > connect methods and then get rid of it. Ok, what if user initialize with something like smtplib.SMTP(source_address=('200.165.12.1', 25)) and calls connect without source_address? |
|
|
| msg129447 - (view) |
Author: Giampaolo Rodola' (giampaolo.rodola) * |
Date: 2011-02-25 22:31 |
| You are right, I haven't thought about that. |
|
|
| msg129496 - (view) |
Author: Paulo Scardine (paulos) |
Date: 2011-02-26 03:44 |
| Giampaolo, Thanks for your kind review, I will send a patch with suggested changes this weekend. Grazie, -- Paulo |
|
|
| msg129699 - (view) |
Author: Paulo Scardine (paulos) |
Date: 2011-02-28 13:24 |
| Patch attached. |
|
|
| msg129701 - (view) |
Author: Paulo Scardine (paulos) |
Date: 2011-02-28 13:25 |
| oops... |
|
|
| msg129703 - (view) |
Author: Paulo Scardine (paulos) |
Date: 2011-02-28 13:32 |
| Sorry for the last post, I had the impression that I forgot to attach the patch. I'm slow this Monday. |
|
|
| msg139343 - (view) |
Author: Paulo Scardine (paulos) |
Date: 2011-06-28 05:07 |
| Is there anything I should improve in order to get this patch commited? |
|
|
| msg141417 - (view) |
Author: Roundup Robot (python-dev)  |
Date: 2011-07-30 02:58 |
| New changeset 26839edf3cc1 by Senthil Kumaran in branch 'default': 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. http://hg.python.org/cpython/rev/26839edf3cc1 |
|
|