-
Notifications
You must be signed in to change notification settings - Fork 598
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
don't use Timeout.timeout() #2147
don't use Timeout.timeout() #2147
Conversation
(outside of `test/`) don't use `Timeout.timeout()` when the underlying library has timeout functionality itself. resolves #975
CHANGELOG for issue 975
When yield phony AWS metadata checks, ignore all options sent to Net::HTTP beyond address and port.
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.
This makes sense to me!
* do not have Net::HTTP override OpenSSL specific timeouts * add TODOs to remind us to clean things up once we specifically only target Ruby versions 2.6 and above
SimpleCov Report
|
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.
Awesome, thanks for explaining everything and adding the todos!
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.
Kudos to @fallwith and @tannalynn for your great discussion about this PR! It helped me a lot during my review.
(outside of
test/
) don't useTimeout.timeout()
when the underlying library has timeout functionality itself.resolves #975