-
Notifications
You must be signed in to change notification settings - Fork 93
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 sockets #35
Conversation
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 see that travis-ci/flake8 check was improperly configured, can you rebase your changes and fix flake8 errors as well?
grpclib/client.py
Outdated
@@ -395,10 +395,24 @@ class Channel: | |||
""" | |||
_protocol = None | |||
|
|||
def __init__(self, host='127.0.0.1', port=50051, *, loop, codec=None): | |||
def __init__(self, host='127.0.0.1', port=50051, path=None, *, loop, codec=None): |
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.
Now host
and port
arguments should be equal to None
by default. Default values (127.0.0.1
and 50051
) should be used if all such arguments are equal to None
. And it would be better to mark path
argument as keyword-only (place it after *
in function's signature).
grpclib/client.py
Outdated
return self._protocol | ||
|
||
async def __connect_tcp__(self): |
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 think that there is no need to create additional functions, especially with "dunder" names, everything can be placed in __connect__
method.
grpclib/server.py
Outdated
@@ -457,7 +457,7 @@ def _protocol_factory(self): | |||
self._handlers.add(handler) | |||
return H2Protocol(handler, self._config, loop=self._loop) | |||
|
|||
async def start(self, host=None, port=None, *, | |||
async def start(self, host=None, port=None, path=None, *, |
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.
It would be better to mark path
argument as keyword-only (place it after *
in function's signature).
grpclib/server.py
Outdated
reuse_address=reuse_address, reuse_port=reuse_port | ||
) | ||
if path is not None: | ||
self._tcp_server = await self._loop.create_unix_server( |
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.
Now _tcp_server
property should be renamed, maybe into just _server
c00a24d
to
c61c15b
Compare
Awesome! Thanks! |
Very soon I'm going to publish a new release candidate, so you will be able to |
This PR adds support for UNIX domain sockets to the
Server
andChannel
.I wasn't exactly sure if there was a preference on how to handle the method signatures, so I implemented this the simplest possible way, but I am happy to refactor if necessary.