Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Consider use over a Unix domain socket #136

Closed
jen20 opened this issue Nov 12, 2019 · 6 comments · Fixed by #184
Closed

Consider use over a Unix domain socket #136

jen20 opened this issue Nov 12, 2019 · 6 comments · Fixed by #184
Milestone
0.1

Comments

@jen20
Copy link
Collaborator

jen20 commented Nov 12, 2019

As discussed in Discord, it would be useful if Tonic could be used via transports other than TCP sockets - for example a Unix domain socket.
@ebkalderon
Copy link

ebkalderon commented Nov 13, 2019 •
edited
Loading

I was just about to open an issue for this. I'm currently using this working branch of hyperlocal to implement a custom Server that serves on Unix domain sockets in a personal project. Converting the client Endpoint, however, is comparatively less trivial.

@LucioFranco
Copy link
Member

I am on vacation right now but ill drop a hint at what I think the correct solution should be. We should provide a MakeConnection builder option for both server and client. This MakeConnection should allow us to swap out the underlying asyncread/write.

@akshayknarayan
Copy link

Over at akshayknarayan@02defcd, I've written a temporary hack which addresses this issue in the same way as TLS support - a "unix" feature gate.

This relies on https://crates.io/crates/hyper-unix-connector, which is similar to hyperlocal except it also provides the plumbing needed to make this connector.

This approach probably should not be merged - in fact, @LucioFranco's suggestion of a MakeConnection builder would probably replace the current tls feature gate as well.

akshayknarayan added a commit to akshayknarayan/tonic that referenced this issue Nov 21, 2019
@akshayknarayan
feat: make Endpoint connector swappable
9a27334 
Change the `connect()` API to lift up the use of `connector()` to
`Endpoint::connect()`, allowing users to provide their own
implementations (for example, Unix-domain sockets).
Any type which impls `tower_make::MakeConnection` is
suitable.

To avoid breaking the default case of HTTP(S), introduce
`connect_with_connector()` and retain `connect()`, which creates the
default connector according to the activated feature gate and passes it
to `connect_with_connector()`.

Fixes: hyperium#136
@akshayknarayan akshayknarayan mentioned this issue Nov 21, 2019
Closed
akshayknarayan added a commit to akshayknarayan/tonic that referenced this issue Dec 3, 2019
@akshayknarayan
feat: make Endpoint connector swappable
36757ba 
Change the `connect()` API to lift up the use of `connector()` to
`Endpoint::connect()`, allowing users to provide their own
implementations (for example, Unix-domain sockets).
Any type which impls `tower_make::MakeConnection` is
suitable.

To avoid breaking the default case of HTTP(S), introduce
`connect_with_connector()` and retain `connect()`, which creates the
default connector according to the activated feature gate and passes it
to `connect_with_connector()`.

Fixes: hyperium#136
@danieleades
Copy link

I was just about to open an issue for this. I'm currently using this working branch of hyperlocal to implement a custom Server that serves on Unix domain sockets in a personal project. Converting the client Endpoint, however, is comparatively less trivial.

there's been a bit of churn on hyperlocal. see

what impact does this have on what you're proposing?

@akshayknarayan
Copy link

Changes in tonic to allow custom connectors in Endpoint are still necessary - the changes in hyperlocal would have impact on callers passing in custom connectors, but not directly here.

@LucioFranco
Copy link
Member

Hey just to update I'd like to get this in tonic for 0.1.

@LucioFranco LucioFranco added this to the 0.1 milestone Dec 6, 2019
@curiousleo curiousleo mentioned this issue Dec 11, 2019
Merged
5 tasks
@LucioFranco LucioFranco mentioned this issue Dec 13, 2019
Merged
LucioFranco added a commit that referenced this issue Dec 13, 2019
@LucioFranco
feat(transport): Allow custom IO and UDS example (#184)
b90c340 
Closes #136
rabbitinspace pushed a commit to satelit-project/tonic that referenced this issue Jan 1, 2020
@LucioFranco @rabbitinspace
feat(transport): Allow custom IO and UDS example (hyperium#184)
70b12c0 
Closes hyperium#136
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
0.1
Development

Successfully merging a pull request may close this issue.

feat(transport): Allow custom IO and UDS example hyperium/tonic
5 participants
@ebkalderon @jen20 @akshayknarayan @LucioFranco @danieleades