-
Notifications
You must be signed in to change notification settings - Fork 30
feat!: Replace hyper-specific interfaces with generic trait #104
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
base: feat/hyper-as-feature
Are you sure you want to change the base?
feat!: Replace hyper-specific interfaces with generic trait #104
Conversation
|
|
||
| // Simple reqwest-based transport implementation | ||
| #[derive(Clone)] | ||
| struct ReqwestTransport { |
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 is temporary until we implement the hyper based version. But it keeps the tests passing without adding new dependencies.
| @@ -1,54 +0,0 @@ | |||
| use futures::{Stream, TryStreamExt}; | |||
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.
Will restore this in a subsequent commit.
| url: Uri, | ||
| headers: HeaderMap, | ||
| reconnect_opts: ReconnectOptions, | ||
| connect_timeout: Option<Duration>, |
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.
These are features of the hyper transport, not part of the core library per se. So these will be brought back in the next PR as part of the hyper transport builder.
Happy to discuss alternatives if you would prefer, but this felt cleaner.
| /// let client = ClientBuilder::for_url("https://example.com/events")? | ||
| /// .build_with_transport(transport); | ||
| /// ``` | ||
| pub fn build_with_transport<T>(self, transport: T) -> impl Client |
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.
All these build methods are really to allow us to get at hyper's internals. We move them to that transport builder, and instead keep this interface simple.
| * **Redirect following** - Automatic handling of HTTP redirects | ||
| * **Last-Event-ID** - Resume streams from last received event | ||
|
|
||
| ## Migration from v0.16 |
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.
I change this later from migration guide to just examples.
This first commit removes hyper as a direct dependency from this crate. Instead, we expose more generic types which we will implement in a subsequent PR.
As this isn't feature complete, I am merging this into a feature branch.