Merge lp:~elachuni/piston-mini-client/timeouts into lp:piston-mini-client
Proposed by
Anthony Lenton
Status: | Merged | ||||||||
---|---|---|---|---|---|---|---|---|---|
Approved by: | Anthony Lenton | ||||||||
Approved revision: | 55 | ||||||||
Merged at revision: | 53 | ||||||||
Proposed branch: | lp:~elachuni/piston-mini-client/timeouts | ||||||||
Merge into: | lp:piston-mini-client | ||||||||
Diff against target: |
268 lines (+137/-8) 6 files modified
doc/envvars.rst (+3/-0) doc/tuning.rst (+17/-0) piston_mini_client/__init__.py (+31/-8) piston_mini_client/consts.py (+1/-0) piston_mini_client/failhandlers.py (+12/-0) piston_mini_client/tests/test_timeout.py (+73/-0) |
||||||||
To merge this branch: | bzr merge lp:~elachuni/piston-mini-client/timeouts | ||||||||
Related bugs: |
|
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
software-store-developers | Pending | ||
Review via email: mp+100417@code.launchpad.net |
Description of the change
This branch adds code for configuring and handling socket timeouts, to fix bug #937865
While I was there, I also wrapped other socket exceptions so that all errors are raised as APIError (or subclasses of that).
To post a comment you must log in.
Thanks for looking into this Anthony.
A few thoughts,
* Does the timeout span the whole request & response, or just the time it takes to get a connected socket? If the former, then a timeout per request would be very helpful.
* Have you had a chance to test this with a pathological endpoint as described in bug 937865? I don't know off the top of my head how to make such a thing, but would be happy to figure it out together with you. I don't think such a thing needs to be added to the automatic test suite, but it would be good to have at least once-off verification that the patch achieves its purpose.
* It would be helpful if the timeout exception included the time taken and a pointer to some docs on how to change the timeout.
Thanks again,
jml