RSDK-2506 Sessions client #301

maximpertsov · 2023-05-25T21:03:08Z

Implements client-side sessions for the python sdk

TODO

Ensure in-progress actions are cancelled if the SDK is forcefully killed

Manual Testing

Copy this branch locally and cd into the repository.
Copy the contents of https://gist.github.com/maximpertsov/acd5d9ddaaa835a000a310341cdfb1de into testing.py (or run the curl script below)

curl https://gist.githubusercontent.com/maximpertsov/acd5d9ddaaa835a000a310341cdfb1de/raw/0fe5f13d1cafef0eb39f7bfcb59556f7f21a53af/viam-python-sessions-testing.py > testing.py

Find your favorite robot with a drive-able base and replace the URI, SECRET, BASE_NAME constants in script above with the relevant parameters.
Run the following commands

poetry add -D aioconsole  # needed for the script above
make install && poetry run python testing.py

You should see output that looks like this:

2023-05-25 17:21:09,768         DEBUG   viam.rpc.dial (dial.py:188)     Creating new viam-rust-utils runtime
2023-05-25 17:21:09,770         DEBUG   viam.rpc.dial (dial.py:209)     Dialing <URI> using viam-rust-utils library
2023-05-25 17:21:11,492         INFO    viam.rpc.dial (dial.py:239)     Connecting to socket: <SOCKET>
2023-05-25 17:21:11,624         DEBUG   viam.sessions_client (sessions_client.py:128)   sent heartbeat successfully
Move in a direction (WASD) or Q to exit
2023-05-25 17:21:12,038         DEBUG   viam.sessions_client (sessions_client.py:128)   sent heartbeat successfully
2023-05-25 17:21:12,450         DEBUG   viam.sessions_client (sessions_client.py:128)   sent heartbeat successfully
2023-05-25 17:21:12,865         DEBUG   viam.sessions_client (sessions_client.py:128)   sent heartbeat successfully
2023-05-25 17:21:13,279         DEBUG   viam.sessions_client (sessions_client.py:128)   sent heartbeat successfully
...

Pressing W A S or D + ENTER should drive the base without any noticeable lag.

edaniels · 2023-05-26T11:04:29Z

why isn't this being implemented in rust-utils?

maximpertsov · 2023-05-26T14:04:50Z

why isn't this being implemented in rust-utils?

the python sdk can still dial a robot directly without using rust-utils, so we need to implement it here to support sessions for that kind of connection.

edaniels · 2023-05-26T14:10:30Z

why isn't this being implemented in rust-utils?

the python sdk can still dial a robot directly without using rust-utils, so we need to implement it here to support sessions for that kind of connection.

got it

edaniels

LGTM assuming testing was done that the robot stopped the base once the python SDK was forcefully killed without warning

edaniels · 2023-05-26T16:16:00Z

src/viam/sessions_client.py

+        if self._disabled:
+            return
+
+        if event.method_name in [self.client.StartSession.name]:


do we need to exempt the other methods at https://github.com/viamrobotics/rdk/blob/main/session/doc.go#L71?

ah didn't see this before, will add

maximpertsov · 2023-05-26T16:45:53Z

LGTM assuming testing was done that the robot stopped the base once the python SDK was forcefully killed without warning

Yeah, let me try this with a longer-running command to confirm

EDIT: done - works as expected. If you run the following script and abort it, the robot will stop moving straight.

# imports / constants ...

async def connect():
    creds = Credentials(type="robot-location-secret", payload=SECRET)
    opts = RobotClient.Options(refresh_interval=0, dial_options=DialOptions(credentials=creds), log_level=DEBUG)
    return await RobotClient.at_address(URI, opts)

async def main():
    robot = await connect()
    base = Base.from_robot(robot, BASE_NAME)
    await base.move_straight(10000, 100)

if __name__ == "__main__":
    asyncio.run(main())

njooma

Just looked over for code quality, didn't actually test.

Looks good! Mostly nits

njooma · 2023-05-26T19:02:03Z

src/viam/sessions_client.py

+)
+
+
+async def delay(coro, seconds):


there's a loop.call_later function that might be useful here:

loop = asyncio.get_running_loop() loop.call_later(seconds, coro)

This looked promising, but unfortunately loop.call_later takes a sync callback and not a co-routine: https://docs.python.org/3/library/asyncio-eventloop.html#asyncio.loop.call_later

njooma · 2023-05-26T19:08:35Z

src/viam/sessions_client.py

+        if self._lock.locked():
+            return
+
+        LOGGER.debug("resetting session")


[nit] can we make all logs Sentence case please?

njooma · 2023-05-26T19:12:23Z

src/viam/sessions_client.py

+                    raise
+            else:
+                if response is None:
+                    raise GRPCError(status=Status.INTERNAL, message="expected response to start session")


Same with error messages as with logs: Sentence case please

njooma · 2023-05-26T19:15:04Z

pyproject.toml

 mypy-protobuf = "^3.4.0"
 tox = "^4.5.1"
 isort = "^5.12.0"
+pytest-watcher = "^0.2.6"


[q] is this needed?

Not strictly, but it was helpful to have this utility when I was spiking out unit tests. I can you remove if you prefer.

Went ahead and removed

maximpertsov · 2023-06-06T19:24:20Z

LGTM assuming testing was done that the robot stopped the base once the python SDK was forcefully killed without warning

Yeah, let me try this with a longer-running command to confirm

EDIT: done - works as expected. If you run the following script and abort it, the robot will stop moving straight.
# imports / constants ...

async def connect():
    creds = Credentials(type="robot-location-secret", payload=SECRET)
    opts = RobotClient.Options(refresh_interval=0, dial_options=DialOptions(credentials=creds), log_level=DEBUG)
    return await RobotClient.at_address(URI, opts)

async def main():
    robot = await connect()
    base = Base.from_robot(robot, BASE_NAME)
    await base.move_straight(10000, 100)

if __name__ == "__main__":
    asyncio.run(main())

For posterity, you should use base.set_power instead of base.move_straight for this test - set_power runs forever.

maximpertsov added 25 commits May 23, 2023 17:56

add test watcher

c27f1a1

spike session client

235a7bf

remove unused

4992c05

client testing

f6da55a

revert debug

1ed1298

send heartbeats faster than interval

7cd5139

init sessions client in viam channel

beaa5bf

wip: pass metadata into request in viam channel

fb2ff54

fixup

b48a9a2

make inner metadata property sync

5063724

reset session when intercepting expiration event

4e9ab91

wip

7720fcf

reset session on close

66aa2d9

fixup expiration check

d9eda25

5%

19919ee

allow sessions to be disabled

0ad2f5d

do not send session metadata for session-specific calls

327d56a

lock

622c9ba

change option

ce86b42

shorter beat interval

0222499

async lock + only acquire lock at startup

b4ad97a

send session metadata with heartbeat

c4396b9

reduce debug logs

3ef9be1

cleanup flow

cb886a6

Merge branch 'main' into sessions-python-RSDK-2506

6843d96

maximpertsov marked this pull request as ready for review May 25, 2023 21:22

maximpertsov requested a review from a team as a code owner May 25, 2023 21:22

maximpertsov requested review from clintpurser and benjirewis and removed request for clintpurser May 25, 2023 21:22

maximpertsov removed the request for review from benjirewis May 25, 2023 21:23

maximpertsov marked this pull request as draft May 25, 2023 21:24

make session_id and supported field internal

a5f448d

maximpertsov marked this pull request as ready for review May 25, 2023 21:31

maximpertsov requested review from edaniels and njooma May 25, 2023 21:31

remove unused

afaff1b

edaniels approved these changes May 26, 2023

View reviewed changes

CR@edaniels exclude methods from receiving session metadata

9f0c4e0

njooma approved these changes May 26, 2023

View reviewed changes

maximpertsov added 3 commits May 30, 2023 09:02

CR@njooma: sentence case logs/errors

bf95c3a

remove pytest-watcher

cf91a1f

add sessions disabled test

50c6bda

maximpertsov merged commit 5451132 into viamrobotics:main May 30, 2023

maximpertsov deleted the sessions-python-RSDK-2506 branch May 30, 2023 14:23

RSDK-2506 Sessions client #301

RSDK-2506 Sessions client #301

Uh oh!

Conversation

maximpertsov commented May 25, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

TODO

Manual Testing

Uh oh!

edaniels commented May 26, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

maximpertsov commented May 26, 2023

Uh oh!

edaniels commented May 26, 2023

Uh oh!

edaniels left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

maximpertsov commented May 26, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

njooma left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

maximpertsov commented Jun 6, 2023

Uh oh!

Uh oh!

maximpertsov commented May 25, 2023 •

edited

Loading

edaniels commented May 26, 2023 •

edited

Loading

maximpertsov commented May 26, 2023 •

edited

Loading