[issue14134] xmlrpc.client.ServerProxy needs timeout parameter

[Demian Brecht]

Demian Brecht added the comment:

>  socket.setdefaulttimeout([timeout]) -- it is bad practice

I'm not really arguing this. It solves the problem, but definitely not in the best of ways. My point in referencing setdefaulttimeout is that if /all/ you care about is the timeout and you're horribly opposed to using an API as I suggested, you still have the option of using setdefaulttimeout. To be clear, it's not something that I'd advocate.

> because setting this global varible we may spoil other cases. example "TCP keepalive" [s.setsockopt(socket.SOL_SOCKET, socket.SO_KEEPALIVE, true) ]

I'm afraid you've somewhat lost me here. Calling setdefaulttimeout simply sets the value of defaulttimeout in socketmodule.c, which newly created sockets use as their default value until explicitly overridden via settimeout().

> and again -- compare what shorter (and what more clear):

Sure, your example is clear for this /specific/ use case. To illustrate my point about ambiguity though, it's unclear what the behaviour should be here based on your patch:

    proxy = ServerProxy('http://example.com/gateway/', timeout=42, transport=Transport(2))

Should the value in the Transport instance be used or the one in the ServerProxy parameter list? It's not obvious and therefore not a sound decision from an API design standpoint.

Ignoring my suggestion of a connection_factory altogether, limiting the timeout to the Transport class is a little more sane and doesn't risk ambiguity:

    proxy = ServerProxy('http://example.com/gateway/', transport=Transport(timeout=2))

Should my suggestion be tossed out, I do think that this is the most sane path to go down.


Now, re-introducing my thoughts on a connection factory (which I still believe is /much/ more flexible and extensible than simply passing in a timeout parameter for the reasons I previously mentioned), should the timeout parameter still be accepted at the Transport level, you run into the same level of ambiguity:

    transport = Transport(timeout=2, connection_factory=lambda h: HTTPConnection(h, timeout=42))

So now the argument in my mind becomes: Should the connection attributes be assignable through Transport instantiation or should the user have accessibility to create their own instance of a connection class that adheres to the expected API and pass that in? To me, the flexibility and benefits of the latter far outweighs the additional complexity:

    transport = Transport(timeout=2)
    transport = Transport(connection_factory=lambda h: HTTPConnection(h, timeout=42))


> if you NOT point timeout in "RPC-client" -- you program will freeze or will maked resource leak (with small probability).

Assuming a lack of concurrency, your program will indeed freeze until the system timeout has been reached. I'm not sure about a leak. If you have an example of such a case, it would likely be a good candidate for a new issue.


> answer: because *unknown* which value need to using by default.

No, the default should be set to socket._GLOBAL_DEFAULT_TIMEOUT (the same default used by HTTPConnection). When unset, the timeout on the socket is unspecified resulting in a value of -1, which then defaults to the system-specific timeout.


Hopefully I've clarified things a little better. Of course, that doesn't mean that you'll agree, in which case we'll just have to agree to disagree :)

----------

_______________________________________
Python tracker <report at bugs.python.org>
<http://bugs.python.org/issue14134>
_______________________________________