-
Notifications
You must be signed in to change notification settings - Fork 49
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 a hyper
client implementation
#32
Conversation
Allows using this client facade with a `tokio` executor
Also, opened the PR before noticing #1, but this should fix it. |
I definitely am! -- I've had frustrating experiences with Hyper and Tokio in the past, to the point that I found it easier to work on a new HTTP stack and runtime. But if this is something that you'd be willing to implement and maintain, then it's most definitely welcome! |
Glad to hear. I have some other libraries I'm working on, and the goal is to make sure that there's meaningful cross-executor support. Happy to help maintain this, I'll get it cleaned up this morning and post another comment once it's ready for a more thorough inspection. |
hyper
client implementationhyper
client implementation
This is ready for review once a maintainer has availability. |
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.
http-types
has a hyperium-http
feature that implements most of the conversions in this PR for you.
An overview of the conversions is here. I believe this covers most of it, but if we're missing something we can update http-types
. That should cover some of the gnarlier bits of this PR (:
My concern with Long story short - happy to use |
Minor note after conversion: the header conversion functions aren't public and because of body type mis-matches they can't be accessed as part of the |
Just wanted to check - is there anything else that needs to be done here? To the best of my knowledge, this is ready for review/merge. |
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 looks great; thanks so much!
Given http-rs/async-h1#109, I don't think it's possible to add
tokio
as an executor for async-h1. Instead, we can addhyper
as a client to thehttp-client
facade, allowing for libraries built on top ofhttp-client
to be executor independent.I still need to do come cleanup and testing, but I wanted to present the work thus far and ask:
tokio
-based client?HttpTypesResponse::try_from()
)