RSDK-7229 add machine_id to cloud metadata #585
Conversation
maximpertsov
force-pushed
the
RSDK-7229-machine-id
branch
from
ace4005 to
34bd29a
Compare
src/viam/robot/client.py
Outdated
| @dataclass | ||
| class CloudMetadata: | ||
| """App-related information about the robot""" | ||
|
|
||
| primary_org_id: str | ||
| location_id: str | ||
| machine_id: str | ||
| machine_part_id: str |
There was a problem hiding this comment.
I would rather use a TypeAlias here rather than creating a new type, especially since everything simply mirrors the GetCloudMetadataResponse anyway
There was a problem hiding this comment.
would making a TypeAlias expose the robot_part_id field on GetCloudMetadataResponse too? this is a duplicate of machine_part_id - we left it to prevent a breaking proto change
There was a problem hiding this comment.
Yes it would. So if you're trying to hide that field, then sure you can create a dataclass. But this is a brewaking change in all the SDKs anyway, so maybe might as well make it a breaking proto change?
There was a problem hiding this comment.
that's fair - I guess there was a feeling that breaking protos is a bit more significant than breaking the client method
There was a problem hiding this comment.
discussed IRL - we're gonna explicitly deprecate the proto field but leave it in: viamrobotics/api#487
Breaking change!Not anymore!Add
machine_idto theget_cloud_metadatarobot client method.Also change the return type and renameWe are just deprecating the duplicate field instead.robot_part_idtomachine_part_idfor clarity.Depends on: