-
Notifications
You must be signed in to change notification settings - Fork 9
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
Use a different process for each device's grpc connection #324
base: master
Are you sure you want to change the base?
Conversation
forch/device_report_client.py
Outdated
break | ||
try: | ||
if not progress or self._convert_and_handle(mac, progress): | ||
print('Progress complete for %s' % mac) |
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.
Debug prints left by mistake?
@@ -50,30 +55,48 @@ def __init__(self, result_handler, target, unauth_vlan, tunnel_ip): | |||
|
|||
def start(self): | |||
"""Start the client handler""" | |||
grpc.channel_ready_future(self._channel).result(timeout=CONNECT_TIMEOUT_SEC) | |||
self._stub = SessionServerStub(self._channel) | |||
# Context may be set already |
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 not sure what to make of this comment. Can you expand it a bit or position it better? Is this to say that if the context is already set then this will throw a RuntimeError? If so, I'd either clarify that in the comment or move to the except clause.
GRPC only maintains 1 single channel (connection to the k8s IBL) in each process so LB across all testing devices can't happen. This PR, with no functional change, ensures each device can have its own connection to the IBL for proper LB.