-
Notifications
You must be signed in to change notification settings - Fork 239
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.
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
Add HTTP open_timeout #134
Conversation
# for various operations. | ||
# | ||
class Client | ||
attr_accessor :timeout, :logger | ||
attr_accessor :open_timeout, :read_timeout, :logger |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we name these as http_open_timeout
and http_read_timeout
to avoid ambiguity?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No problem. Do you still need @timeout
parameter?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, let's leave it there as it is used in the System
class.
Also the CI build is failing because the newly added attributes are not mocked yet, could you please fix that as well if you get a chance? |
Hi! Is that good for you? |
Looks great! |
Released as |
Great! Thank you! |
If the url is wrong or not responding, timeout after
open_timeout
.Keep it separated from
read_timeout
.