Add request timeout value for all request methods#40
Add request timeout value for all request methods#40rwnx wants to merge 1 commit intoalgorithmiaio:developfrom rwnx:master
Conversation
This is a sensible thing, but specifically recommended by requests https://2.python-requests.org/en/master/user/quickstart/#timeouts > Nearly all production code should use this parameter in nearly all requests. > Failure to do so can cause your program to hang indefinitely this was introduced in requests 2.4.0 so i've added a version constraint to setup.py to support this. See https://requests.readthedocs.io/en/latest/community/updates/#id55
zeryx
left a comment
There was a problem hiding this comment.
Hey @jerometwell ! Thanks for the PR!
We typically provide a mechanism for the API requests themselves (especially algorithm requests) to have server side timeouts (https://algorithmia.com/developers/api/?python#invoke-an-algorithm); however I can understand the reasoning for why you may want to be able to set a timeout with the CLI.
I'd suggest in keeping with our current infrastructure design to maintain the server side timeout system, and to provide that timeout=... flag in the event that a user passes such a flag to the algo run .. operation.
With that kind of change I think that would make a great addition to our CLI and I'd be happy to merge it in :)
|
Hi @zeryx , Thanks for responding. This won't interfere with the current timeout system at all - it has an entirely different function that relates to the socket behaviour and the raw http-y parts of this client. I didn't really have any intention of it being configurable by the caller. My Intention was to set a sensible default for general network conditions so the library will not block indefinitely on a bad network connection/bad socket. As noted by the creator of the |
This is a sensible thing, but specifically recommended by requests
https://2.python-requests.org/en/master/user/quickstart/#timeouts
this was introduced in requests 2.4.0 so i've added a version constraint
to setup.py to support this.
See https://requests.readthedocs.io/en/latest/community/updates/#id55
I'm using a constant in
Algorithmia/clientto do this. I want to make it clear that it's static config at the moment so as to keep a single source of truth.