Added configurable connection timeouts #187
Conversation
zanker
force-pushed
the
timeouts
branch
2 times, most recently
from
a830eee
to
15bf359
Compare
|
I guess this will close #160 |
|
Yup! This is kind of a complicated set of changes though. See any issues with it? |
|
@zanker It looks like the “HTTP::Client timeouts with a per operation timeout connection of 0 times out” specs are failing in CI. |
|
Haha awesome. I can make it go green locally, but I expect the CI servers are running fast enough that it immediately connects, or maybe they do something strange. I don't have a good way of testing connection latency. Since it's "connected" before I call accept on a server, and if the servers not up it just fails immediately. Let me try setting a float timeout and see if that hacks it into working. |
|
Timeouts, especially ones related to I/O so they can't simply be mocked, have been rather troublesome for me to test in things like Celluloid. Short of using something like rspec-retry and rerunning the test until it passes, I'm not sure I have any good advice. On Celluloid we use a "quantum" of 50ms for all timer tests. |
|
:( I stripped that test then |
|
Seems there are still 3 failing tests... and a rebase is needed |
|
Think something got a bit messed up from merge conflicts. Should go green now |
|
Awesome!!! 👏 |
Added configurable connection timeouts
|
Released as 0.8.0.pre4 |
This is mildly crazy. Basically this adds three sets of timeouts
Null, which is just what there is today (nothing)
Per operation, which is what you see in things like Net::HTTP, Excon where the timeout is per operation
Global, which sets an allocation of how much time can be used and uses it up as the request goes. Ensuring a request can never run longer than the set time.
Getting DNS to have a customizable timeout is awful though, which is why there's a lot of extra code for doing
connect(mostly cribbed from Tony's code). My inclination, would be to just wrap it inTimeout.timeoutsince that can be done more safely than relying on all the Ruby internals to handle the DNS/open timeouts properly. And it's safer, any thoughts on that?