Idea #7697
closed
[SDKs] Improve socket error handling in PySDK
Added by Brett Smith about 9 years ago.
Updated about 9 years ago.
Description
History: we added brute-force socket error retry handling in #7587. We want to improve it to use the same rules we're using in the Go SDK in #5538. Modify _intercept_http_request to do the following:
- If we're about to do a POST or DELETE, and (as far as we know) the http client's socket has been idle for more than 30 seconds, close its socket. (httplib2 will automatically open a new one to make the request.) It might be hard to tell how long the socket has been idle: "we haven't used the http client for 30 seconds, and the socket is idle right now" might be a good enough approximation.
- Retry BadStatusLine and socket.error the same way as we currently do, but only if the original request was idempotent (i.e., HEAD, GET, or PUT).
- Description updated (diff)
- Description updated (diff)
- Target version set to Arvados Future Sprints
- Description updated (diff)
- Target version changed from Arvados Future Sprints to 2015-12-02 sprint
- Assigned To set to Tom Clegg
- Status changed from New to In Progress
54fc73b just has one small bug that should get fixed before merge: except httplib.BadStatusLine, httplib.HTTPException: should just say except httplib.HTTPException:. What you wrote will catch httplib.BadStatusLine exceptions and bind any exception caught to httplib.HTTPException. BadStatusLine inherits from HTTPException, so we can just catch that.
Beyond that, I just have small readability suggestions, none of which are necessary for merge:
- The
import Queue added to test_api.py seems unused.
- Two blank lines before the RetryREST definition would be nicely PEP 8-compliant.
self.api._http.orig_http_request.side_effect = lambda x, *a, **k: self.request_success I think can just be self.api._http.orig_http_request.return_value = self.request_success. side_effect takes precedence over return_value, so test methods that override that will still work as intended.- You can DRY up _test_connection_close by changing
self.api._http.connections = {str(i): mock_conns[i] for i in range(2)} to self.api._http.connections = {str(i): c for i, c in enumerate(mock_conns)}.
Thanks.
- Status changed from In Progress to Resolved
Applied in changeset arvados|commit:96a3d13e88c67f6beef17877f15d97a03c63b525.
- Related to Bug #12684: Let user specify a retry strategy on the client object, used for all API calls added
- Related to deleted (Bug #12684: Let user specify a retry strategy on the client object, used for all API calls)
- Related to Bug #12684: Let user specify a retry strategy on the client object, used for all API calls added
Also available in: Atom
PDF
Updated by 
Updated by