-
Notifications
You must be signed in to change notification settings - Fork 113
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
client: add timer() option #25
Conversation
61060ba
to
980215e
Compare
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.
What do you mean it fails without a timer? That should be fixed if that's the case, a timer shouldn't be required.
Right now I am getting a panic here https://github.com/hyperium/hyper/blob/9ed175d1545fa338b6212c4f48be9b3d12821c7a/src/common/time.rs#L61 called from Client::send_request, which eventually calls I am not seeing where pool has a timer, looks like spawn_idle_interval alwys uses tokio::time::interval if I am reading things right |
Ah, is that because you're setting some of the HTTP/2 keep-alive options? |
5f15fb0
to
ae2d344
Compare
Hey @seanmonstar , just coming back to this. I think this is ready to go from my POV, let me know what you think when you have a chance? |
Currently, a timer must be provided to use http2. However, this has no option to actually configure this. This PR introduces a new function to set the timer. With this, I am able to successfully make HTTP2 calls with the Client.
d7ac38c
to
b0790a6
Compare
Currently, a timer must be provided to use http2. However, this has no option to actually configure this. This PR introduces a new function to set the timer. With this, I am able to successfully make HTTP2 calls with the Client (as long as there is no error - currently we hit a todo!() on errors)