-
Notifications
You must be signed in to change notification settings - Fork 191
draft version of sans io tcp client #2124
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: master
Are you sure you want to change the base?
Conversation
7bc46c9 to
93cb634
Compare
| type Join: Send + 'static; | ||
| type Sleep: Future<Output = ()> + Send + 'static; | ||
|
|
||
| fn spawn(&self, fut: impl Future<Output = ()> + Send + 'static) -> Self::Join; |
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.
Those trait bounds are very strict. I understand that tokio needs those, but other runtimes don't. This is the problem with such abstractions such as the Runtime trait. I saw it in the opentelemetry_sdk crate aswell.
I think we should search for different approach, in which the concrete implementations of sans-io provides those capabilities without polluting the internal state machine of the protocol.
The sleep function is acceptable as it is required for timeouts, but spawning is not.
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.
Yes, you’re right, that’s a good point. We should probably make spawn less strict (maybe by checking how other projects have approached it). As for time, it could be abstracted in the way described in the firezone blog https://www.firezone.dev/blog/sans-io#abstracting-time, while still allowing the ConnectionDriver to poll it directly
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.
Left one comment, so far it looks good, I think we should create an RFC for this feature and initiate a GH discussion. Let's use the rust-lang template for RFC.
|
I really like the idea, great job with the initial draft! I think that it'd be nice to have the different implementations available behind the feature flag. For example, the very basic SDK could only come with a raw, synchronous TCP client using the std library. In order to use e.g. tokio, smol etc., you'd simply need to include e.g. |
Sounds good, I’ll open a discussion |
Yeah, exactly 🙂 That’s what I had in mind as well. But in addition to being runtime-agnostic, we could also make it transport-agnostic, so features would let us pick combinations like ["tokio", "tcp"] or ["tokio", "http"], not just ["tokio"] |
I think that the default SDK, could contain just the sync std TCP client. All the rest, whether it's QUIC, TCP using Tokio, HTTP with reqwest etc. could be the separate features. |
This PR introduces a draft version of a sans-io TCP client. The main goal is to separate protocol logic from the actual I/O layer, so that the client is no longer tightly bound to a specific runtime or transport implementation.
What’s included
Why