RSDK-10366 - support rust utils on windows #900
Conversation
| if sys.platform != "win32" and sys.platform != "cygwin": | ||
| self._dial_options.disable_webrtc = True |
There was a problem hiding this comment.
I was unable to get the SessionsClient to reliably connect and stay connected on windows when webRTC was disabled. Leaving it enabled is a little inefficient insofar as it means we're creating a second rust runtime, but 1) the windows workflow is already a little inefficient because we're using TCP instead of UDS for our proxy, and 2) I figure a minor inefficiency here is better than not supporting a SessionsClient on windows at all. No regression on linux/mac since we leave it disabled when the platform isn't windows.
cc @cheukt, who set up the disabling of webRTC initially here. If there's a particular reason to be concerned about re-enabling, please let me know!
src/viam/sessions_client.py
Outdated
| @@ -37,6 +38,7 @@ class SessionsClient: | |||
| channel: Channel | |||
| client: RobotServiceStub | |||
| _address: str | |||
| _robot_address: Optional[str] | |||
There was a problem hiding this comment.
We could probably get away with not storing this, since only one of _address and _robot_address is ever necessary, but the logic got slightly more finicky and less readable, and the cost of storing the address here is very small, so it's probably not a big deal and not worth flagging here but it was annoying me so I'm articulating it!
There was a problem hiding this comment.
I would perhaps add a comment here differentiating between these 2. It took me a second to figure it out, so would be good to leave breadcrumbs for our future selves
src/viam/sessions_client.py
Outdated
| @@ -37,6 +38,7 @@ class SessionsClient: | |||
| channel: Channel | |||
| client: RobotServiceStub | |||
| _address: str | |||
| _robot_address: Optional[str] | |||
There was a problem hiding this comment.
I would perhaps add a comment here differentiating between these 2. It took me a second to figure it out, so would be good to leave breadcrumbs for our future selves
No description provided.