Re: [Python-Dev] I vote to reject: Adding timeout to socket.py and httplib.py.
[Facundo Batista] Remember that this function primary use is for higher level libraries Yes, I see that clearly now. But remember that by adding a new function to the socket module to support httplib et al, you are also adding a function to the socket module that will be used directly by end users. I vote to reject this patch. The underlying problem it is trying to address is that httplib.HTTPConnection objects do not support timeouts. But solving that problem by moving the functionality of the HTTPConnection.connect() method inside the socket module as a standalone function is the *wrong* solution. The proposed new function does not belong in the socket module. In contrast to all of the other socket creation and management functionality in the socket module, the new function does not handle non-blocking IO. Also, the new functions' use-case is too restricted, to client connections with a positive timeout: this functionality is trivially available using existing functionality, e.g. mysock = socket(AF_INET, SOCK_STREAM) mysocket.settimeout(1.0) mysocket.connect( (host, port) ) # etc In contrast to the new function, the existing functionality in the socket module is much more general, and much better designed to handle the wide range of situations in which sockets are used. Socket APIs are hard to do right because sockets are complex and hard to do right. The problem the patch seeks to fix is is in httplib; the problem should be fixed in httplib. I recommend modifying the patch to remove *all* proposed changes to the socket module. Instead, the patch should restrict itself to fixing the httplib module. Sorry, Alan. ___ 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] I vote to reject: Adding timeout to socket.py and httplib.py.
Alan Kennedy wrote: The proposed new function does not belong in the socket module. In contrast to all of the other socket creation and management functionality in the socket module, the new function does not handle non-blocking IO. The rest of the socket module isn't going anywhere. If you want to do non-blocking IO, don't use the convenience function that is designed for use with blocking IO (even better, use a library that makes non-blocking IO easier to live with, like asyncore or Twisted). While the name of the proposed function and it's signature need to change (which Facundo has already acknowledged), correctly looking up an address and attempting to connect to it is a non-trivial operation, but one that is duplicated in several stdlib modules (httplib, poplib, ftplib, smtplib at least). This level of repetition for a dozen line function is insane - Facundo's patch is merely the first step towards standardising it. Cheers, Nick. -- Nick Coghlan | [EMAIL PROTECTED] | Brisbane, Australia --- http://www.boredomandlaziness.org ___ 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] I vote to reject: Adding timeout to socket.py and httplib.py.
Alan Kennedy wrote: But remember that by adding a new function to the socket module to support httplib et al, you are also adding a function to the socket module that will be used directly by end users. I vote to reject this patch. Well, you can vote to name it _create_connection(), if your concern is what the end user will do with it. Or it's just denial towards this patch? I recommend modifying the patch to remove *all* proposed changes to the socket module. Instead, the patch should restrict itself to fixing the httplib module. -1 to repeat the same functionality in 5 other libraries. As I said above, we can make it non-public. So, as a resume of the choices we still need to settle: a) Repeat the same functionality in 5 other libraries b) Write the function in socket.py, public c) Write the function in socket.py, non public We need to make the decission. The functionality was needed years ago, I don't want to lose a year discussing... The way I see it, we have three posible ways: a) python-dev votes b) python-dev doesn't care, nobody votes, I make it my way c) Guido settles this down Voting is open, ;) Regards, -- . Facundo . Blog: http://www.taniquetil.com.ar/plog/ PyAr: http://www.python.org/ar/ ___ 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] I vote to reject: Adding timeout to socket.py and httplib.py.
Facundo Voting is open, ;) ... Facundo So, as a resume of the choices we still need to settle: Facundo a) Repeat the same functionality in 5 other libraries Facundo b) Write the function in socket.py, public Facundo c) Write the function in socket.py, non public I vote 'c'. Skip ___ 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] I vote to reject: Adding timeout to socket.py and httplib.py.
[Facundo] Voting is open, ;) So what are we voting on exactly? The patch as it currently is? The patch has not been updated to reflect recent discussions on the list. Will the patch be updated before the vote? I have one more issue with the patch. I think that the exception handling in the function is wrong. This is (approximately) the way the patch is currently defined. def create_connection(address, **kwargs): msg = getaddrinfo returns an empty list host, port = address for res in getaddrinfo(host, port, 0, SOCK_STREAM): af, socktype, proto, canonname, sa = res sock = None try: [snip] return socket except error, err: msg = str(err) # -- don't do this if sock is not None: sock.close() raise error(msg) Changing the exception to with msg = str(err) is the wrong thing to do: this will hide the exception and make comparisons with symbolic constants fail, for users that want to handle exceptions. The correct way to handle 1. An empty return from getaddrinfo() 2. Other socket errors is as follows def create_connection(address, **kwargs): host, port = address for res in getaddrinfo(host, port, 0, SOCK_STREAM): af, socktype, proto, canonname, sa = res sock = None try: [snip] return new_socket except error, err: if sock is not None: sock.close() raise err else: raise error(getaddrinfo returns an empty list) [Facundo] Or it's just denial towards this patch? I think this patch is poorly designed and poorly implemented. There are multiple problems in its 17 lines of socket module code; every time I look I find a new problem. Sorry. Alan. ___ 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] I vote to reject: Adding timeout to socket.py and httplib.py.
Alan Kennedy wrote: I think this patch is poorly designed and poorly implemented. There are multiple problems in its 17 lines of socket module code; every time I look I find a new problem. Code which is copied verbatim from httplib, and also occurs with slight variations in several other standard library modules. So all these objections indicate to me is that the operation *needs* to be centralised, any issues corrected, and then the corrected version invoked from all of the places in the standard library that need it. httplib just happens to be the first cab off the rank (as was discussed on this list a short time ago). The objection about but it doesn't support non-blocking sockets doesn't make sense to me. This is a function to make creation of a blocking socket more convenient - if you're doing non-blocking IO, why would you even try to use it? Cheers, Nick. -- Nick Coghlan | [EMAIL PROTECTED] | Brisbane, Australia --- http://www.boredomandlaziness.org ___ 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] I vote to reject: Adding timeout to socket.py and httplib.py.
Alan Kennedy wrote: So what are we voting on exactly? The patch as it currently is? The patch has not been updated to reflect recent discussions on the list. Will the patch be updated before the vote? The voting is on a, b or c. The patch will be updated after I know what python-dev want to do. The correct way to handle 1. An empty return from getaddrinfo() 2. Other socket errors is as follows def create_connection(address, **kwargs): host, port = address for res in getaddrinfo(host, port, 0, SOCK_STREAM): af, socktype, proto, canonname, sa = res sock = None try: [snip] return new_socket except error, err: if sock is not None: sock.close() raise err else: raise error(getaddrinfo returns an empty list) Wrong! You're raising an error the first time you can not connect. Your way, the for is superfluous. You're changing functionality here... See? It's not so easy. Maybe I should correct the patch like this: msg = getaddrinfo returns an empty list host, port = address for res in getaddrinfo(host, port, 0, SOCK_STREAM): af, socktype, proto, canonname, sa = res sock = None try: [snip] return sock except error, msg: if sock is not None: sock.close() raise error, msg Problems with this approach: - It raises an exception the old way - If you try to create three sockets, and all with errors, you'll only see the last one. Why I think that this is the way to go: - Because you're not changing the semantics of the function. Right now it's like this, and maybe we break some code if we change it. I think this patch is poorly designed and poorly implemented. There are multiple problems in its 17 lines of socket module code; every time I look I find a new problem. It's better than yours. Talk is cheap. Regards, -- . Facundo . Blog: http://www.taniquetil.com.ar/plog/ PyAr: http://www.python.org/ar/ ___ 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] I vote to reject: Adding timeout to socket.py and httplib.py.
On 3/21/07, Facundo Batista [EMAIL PROTECTED] wrote: So, as a resume of the choices we still need to settle: a) Repeat the same functionality in 5 other libraries b) Write the function in socket.py, public c) Write the function in socket.py, non public The fact that it's needed in 5 places in the stdlib suggests that there's plenty of user code that could benefit from it too. Hence, I prefer (b), but if that really freaks people out, I'm okay with (c) too. I'm not okay with (a). Thanks, by the way, for creating this patch (and updating it based on python-dev feedback). I think it's really important for the maintainability of Python that duplicate code is factored out whenever possible. STeVe -- I'm not *in*-sane. Indeed, I am so far *out* of sane that you appear a tiny blip on the distant coast of sanity. --- Bucky Katt, Get Fuzzy ___ 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] I vote to reject: Adding timeout to socket.py and httplib.py.
[Facundo] Talk is cheap. I'm sorry if you see my attempts to review your patch as cheap talk. Maybe I should have just kept my opinion to myself. You'll get your chance to return the favour when I check in my upcoming 1000+ line change to jython 2.3 to bring the jython socket, select and asyncore modules up-to-date with cpython 2.5, including providing ssl support, non-blocking IO, select/poll, etc, for the first time in jython. Alan. ___ 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] I vote to reject: Adding timeout to socket.py and httplib.py.
[Facundo] Talk is cheap. I'm sorry if you see my attempts to review your patch as cheap talk. Maybe I should have just kept my opinion to myself. You'll get your chance to return the favour when I check in my upcoming 1000+ line change to jython 2.3 to bring the jython socket, select and asyncore modules up-to-date with cpython 2.5, including providing ssl support, non-blocking IO, select/poll, etc, for the first time in jython. Alan. ___ 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] I vote to reject: Adding timeout to socket.py and httplib.py.
Facundo Batista [EMAIL PROTECTED] wrote: Alan Kennedy wrote: I recommend modifying the patch to remove *all* proposed changes to the socket module. Instead, the patch should restrict itself to fixing the httplib module. -1 to repeat the same functionality in 5 other libraries. As I said above, we can make it non-public. So, as a resume of the choices we still need to settle: a) Repeat the same functionality in 5 other libraries b) Write the function in socket.py, public c) Write the function in socket.py, non public b or c is fine, I have no preference. In regards to 'there is no way to create a blocking socket this way', Alan is off his rocker. Facundo has already stated that he would like to use something that will allow for the passing of None as a timeout argument to specify no timeout - aka blocking sockets, as per sock.settimeout(None) (through either **kwargs or timeout=sentinel). The function is needed, and the implementation is sufficient for its intended uses. - Josiah ___ 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] I vote to reject: Adding timeout to socket.py and httplib.py.
[Josiah] In regards to 'there is no way to create a blocking socket this way', Alan is off his rocker. I am not off my rocker. And I never wrote the words you place in quotes (except in relation to an earlier defect in the patch where the timeout=None value was ignored). What I clearly stated is that the function as is doesn't cater for *non-blocking* sockets. I also clearly stated that I have no problem with the fact that it doesn't handle non-blocking sockets, but this either A: Needs to be enforced in the function by disallowing zero timeouts B: Needs to be recorded in the documentation The use cases for the function are limited: that's fine. But either explicitly limit them or document those limits. The function is needed, and the implementation is sufficient for its intended uses. When all the defects are fixed, it will be sufficient. Alan. ___ 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] I vote to reject: Adding timeout to socket.py and httplib.py.
On 3/21/07, Steven Bethard [EMAIL PROTECTED] wrote: On 3/21/07, Facundo Batista [EMAIL PROTECTED] wrote: So, as a resume of the choices we still need to settle: a) Repeat the same functionality in 5 other libraries b) Write the function in socket.py, public c) Write the function in socket.py, non public The fact that it's needed in 5 places in the stdlib suggests that there's plenty of user code that could benefit from it too. Hence, I prefer (b), but if that really freaks people out, I'm okay with (c) too. I'm not okay with (a). I agree with the reasoning leading to choice (b) as optimal. -- --Guido van Rossum (home page: http://www.python.org/~guido/) ___ 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] I vote to reject: Adding timeout to socket.py and httplib.py.
Facundo Batista wrote: So, as a resume of the choices we still need to settle: a) Repeat the same functionality in 5 other libraries b) Write the function in socket.py, public c) Write the function in socket.py, non public or d) Put it in another module Is it time for a sockettools module, maybe? -- Greg ___ 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] I vote to reject: Adding timeout to socket.py and httplib.py.
[EMAIL PROTECTED] wrote: Facundo c) Write the function in socket.py, non public Also this option has the problem that it would be a strange usage of non-public, since the function would be designed for use by other modules and not used at all in the module it's supposedly private to. -- Greg ___ 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] I vote to reject: Adding timeout to socket.py and httplib.py.
On 3/21/07, Alan Kennedy [EMAIL PROTECTED] wrote: [Greg Ewing] or d) Put it in another module Is it time for a sockettools module, maybe? +1! -1. The new module would be just as much a jumble of unrelated APIs as the socket module already is, so there's no particularly good reason to create an arbitrary separation. Also, KISS. -- --Guido van Rossum (home page: http://www.python.org/~guido/) ___ 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] I vote to reject: Adding timeout to socket.py and httplib.py.
[Greg Ewing] or d) Put it in another module Is it time for a sockettools module, maybe? +1! Alan. ___ 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] I vote to reject: Adding timeout to socket.py and httplib.py.
[Facundo] Talk is cheap. I'm sorry if you see my attempts to review your patch as cheap talk. Maybe I should have just kept my opinion to myself. You'll get your chance to return the favour when I check in my upcoming 1000+ line change to jython 2.3 to bring the jython socket, select and asyncore modules up-to-date with cpython 2.5, including providing ssl support, non-blocking IO, select/poll, etc, for the first time in jython. Alan. ___ 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] I vote to reject: Adding timeout to socket.py and httplib.py.
Guido van Rossum wrote: Is it time for a sockettools module, maybe? +1! -1. The new module would be just as much a jumble of unrelated APIs as the socket module already is, so there's no particularly good reason to create an arbitrary separation. Also, KISS. I agree with Guido on this one. Bill ___ 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
