-
-
Notifications
You must be signed in to change notification settings - Fork 188
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 support for unix domain sockets #680
base: master
Are you sure you want to change the base?
Conversation
grpc.NewClient already supports connecting to unix domain sockets, and accepts a string anyways. As a quick fix, detect the `address` starting with `unix://` and don't add the port. In the long term, we might want to deprecate `host` and `port` cmdline args in favor of a single `address` arg.
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## master #680 +/- ##
==========================================
+ Coverage 77.10% 77.13% +0.02%
==========================================
Files 57 57
Lines 3910 3915 +5
==========================================
+ Hits 3015 3020 +5
Misses 647 647
Partials 248 248 |
@ktr0731 poke ;-) |
addr := cfg.Server.Host | ||
|
||
// as long as the address doesn't start with unix, also add the port. | ||
if !strings.HasPrefix(cfg.Server.Host, "unix://") { |
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 would personally add --unix switch and use that to decide if we do host:port or just host
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'm waiting on @ktr0731 to decide on the path forward.
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.
grpc.NewClient already supports connecting to unix domain sockets, and accepts a string anyways.
As a quick fix, detect the
address
starting withunix://
and don't add the port.In the long term, we might want to deprecate
host
andport
cmdline args in favor of a singleaddress
arg.Fixes #654.