Open ArhanChaudhary opened 1 year ago
A couple of preliminary thoughts:
int
will apply to the read
and write
timeouts. Would a tuple
make more sense so the user explicitly has to set both values?CanvasException
to handle the Timeout exception emitted by requests
? Canvas
would be fine to make sure the error is captured correctly.Hello @bennettscience, thanks for your thoughts. I'll commit and address your insights once I get feedback on my response:
exc=None
kwarg to register_uris
and pass that into requests_mocker.register_uri
so ReadTimeout
s can be caught in a with self.assertRaises(ReadTimeout):
block?All modified and coverable lines are covered by tests :white_check_mark:
Project coverage is 100.00%. Comparing base (
4b4fbd8
) to head (3819739
).
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
Proposed Changes
Adds a default API request timeout. I originally wanted to have a timeout keyword argument for every Requester.request call, but there were far too many instances of self._requester.request in the codebase, so I've instead insisted on a global default timeout.
I was unsure how to write the unit test for the mocked request wait and raise a ReadTimeout, so I'll just have to leave that up to a reviewer.
Fixes #629 .