Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Use ReadTimeout as base timeout exception for the requests client #337

Merged
merged 1 commit into from
Dec 7, 2017
Merged

Use ReadTimeout as base timeout exception for the requests client #337

merged 1 commit into from
Dec 7, 2017

Conversation

sjaensch
Copy link
Contributor

@sjaensch sjaensch commented Dec 7, 2017

This fixes issues with code that catches requests.exceptions.ReadTimeout, which we broke by using requests.exceptions.Timeout as base class.

@coveralls
Copy link

coveralls commented Dec 7, 2017 •
edited
Loading

Coverage Status

Coverage increased (+0.003%) to 98.582% when pulling 95f28d3 on sjaensch:use-requests-readtimeout into c524bce on Yelp:master.

@sjaensch sjaensch merged commit fbf429d into Yelp:master Dec 7, 2017
@sjaensch sjaensch deleted the use-requests-readtimeout branch December 7, 2017 12:22
@ngaya-ll
Copy link
Contributor

What about requests.exceptions.ConnectTimeout?

@macisamuele
Copy link
Collaborator

@ngaya-ll : connection errors are not integrated yet because we didn't find a reliable way of testing those type of errors with fido client; but definitely our plan is to "generalize" also those type of errors (#346)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@macisamuele macisamuele macisamuele approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@sjaensch @coveralls @ngaya-ll @macisamuele