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.

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

Allow directly specifiying connection path #5701

Merged
merged 1 commit into from
May 3, 2016
Merged

Allow directly specifiying connection path #5701

merged 1 commit into from
May 3, 2016

Conversation

bluecmd
Copy link
Contributor

@bluecmd bluecmd commented Mar 10, 2016

Make insecure_channel / secure_channel accept port=None and only use the
'host' argument. This enables using UNIX sockets without mangling the
path of the socket.

The API would be clearer if port somehow was optional (like passing a (host, port) tuple like a lot of other Python libraries are using) but to maintain compatibility with the current users this solution is proposed.

This provides relief for #4974

@nathanielmanistaatgoogle
Copy link
Member

This looks great; I will merge it after we exit our test fixit.

@jtattermusch
Copy link
Contributor

this is ok to test

@jtattermusch
Copy link
Contributor

@nathanielmanistaatgoogle for external contributors, you need to use "this is ok to test" magic phrase allow the tests to actually run.

Make insecure_channel / secure_channel accept port=None and only use the
'host' argument. This enables using UNIX sockets without mangling the
path of the socket.

Signed-off-by: Christian Svensson <blue@cmd.nu>
@nathanielmanistaatgoogle
Copy link
Member

@jtattermusch: Does this need to be amended to remove the copyright update since we've changed our license header practices? Or can it be merged in its current state?

@jtattermusch jtattermusch merged commit 442bead into grpc:master May 3, 2016
@lock lock bot locked as resolved and limited conversation to collaborators Jan 27, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants