RSDK-7899 Add MachineStatus to RobotService #527
Conversation
| repeated ResourceStatus resources = 1; | ||
| } | ||
|
|
||
| message ResourceStatus { |
There was a problem hiding this comment.
note that error is currently omitted - it will be added once we implement the "unhealthy" state
|
|
||
| message ResourceStatus { | ||
| enum State { | ||
| STATE_UNSPECIFIED = 0; |
There was a problem hiding this comment.
this state is purely a default - it should never be returned by RDK
| } | ||
|
|
||
| message ResourceStatus { | ||
| enum State { |
There was a problem hiding this comment.
[q] Does the defining the enum inside of ResourceStatus make any difference in the compiled code then if you were to define enum outside of the message?
There was a problem hiding this comment.
Changing the nesting level shouldn't be a breaking change based on bufbuild/buf#400
If the previous and current types are both enums, the enums are checked to see if the (1) the short names are equal (2) the previous enum is a subset of the current enum. A subset is defined as having a subset of the name/number enum values. If the previous is a subset, no failure is produced. The idea here is that this covers if someone just moves where an enum is defined, but still allows values to be added to this enum in the same change, as adding values to an enum is not a breaking change.
However, it's worth noting that this was not the case before: bufbuild/buf#80
There was a problem hiding this comment.
Cool; thanks for looking into it!
| STATE_READY = 3; | ||
| // a resource that is being removed from the robot. | ||
| STATE_REMOVING = 4; | ||
| } |
There was a problem hiding this comment.
I wonder if adding STATE_UNHEALTHY eventually will be a breaking change. Not the end of the world, but curious how enum modifications are handled (i.e. if the "indices" of the other variants don't change, maybe it's not breaking).
There was a problem hiding this comment.
This shouldn't be breaking based on my read of https://protobuf.dev/programming-guides/enum/#definitions.
The key point is that proto3 treats enums as "open", which means that values outside of the enum range are set directly on the field. So an old client can receive messages with this field containing values that are outside of the defined enum range.
There was a problem hiding this comment.
which is great, since we might add new states beyond unhealthy in the future
There was a problem hiding this comment.
Awesome, that's my reading, too.
maximpertsov
changed the title
maximpertsov
added
safe to test
Co-authored-by: Benjamin Rewis <32186188+benjirewis@users.noreply.github.com>
maximpertsov
force-pushed
the
RSDK-7899-machine-status
branch
from
213a895
to
606ed7f
Compare
maximpertsov
added
ready-for-protos
| @@ -15,7 +15,7 @@ import ( | |||
|
|
|||
| "github.com/grpc-ecosystem/grpc-gateway/v2/runtime" | |||
| "github.com/grpc-ecosystem/grpc-gateway/v2/utilities" | |||
| "go.viam.com/api/common/v1" | |||
| v1_0 "go.viam.com/api/common/v1" | |||
There was a problem hiding this comment.
Not a blocking question (it's been merged obviously!), but do we know why this happened?
There was a problem hiding this comment.
my understanding is that the formatter is unstable
There was a problem hiding this comment.
Got it, so this is just how your local code generator behaved. Thanks!
* main: (22 commits) DATA-3094: Add CLI args to SubmitCustomTrainingJobRequest (#550) bump min golang version (#548) DATA-3054: Added UpdateBoundingBox (#547) RSDK-8595 update docs with new behavior (#546) APP-3604 build endpoints for external oauth and repos (#544) add last updated to fragment proto (#540) APP-5851: Add optional visibility field to `CreateFragmentRequest` (#543) APP-5314 - add dates and limit to GetRobotPartLogsRequest (#538) RSDK-7903 Add unhealthy state and error field (#539) RSDK-8381 add viam server version to API (#537) [APP-5417]: Undo APP-5417 (#536) [APP-5417]: Accommodate pagination for GetRobotPartHistory in api (#535) Data 2816/get training job logs (#534) DATA-2792 - Add URL to UpdateRegistryItemRequest, but make it optional (#532) RSDK-8228 Add config status to machine status API (#531) Add revision field to config (#530) [APP-5420] Add ListMachineFragments request and response (#525) [APP-5400]: Add GetFragmentHistory proto (#524) RSDK-7899 Add MachineStatus to RobotService (#527) RSDK-8083: add LogPatternConfig field to RobotConfig API (#526) ...
* main: (22 commits) DATA-3094: Add CLI args to SubmitCustomTrainingJobRequest (#550) bump min golang version (#548) DATA-3054: Added UpdateBoundingBox (#547) RSDK-8595 update docs with new behavior (#546) APP-3604 build endpoints for external oauth and repos (#544) add last updated to fragment proto (#540) APP-5851: Add optional visibility field to `CreateFragmentRequest` (#543) APP-5314 - add dates and limit to GetRobotPartLogsRequest (#538) RSDK-7903 Add unhealthy state and error field (#539) RSDK-8381 add viam server version to API (#537) [APP-5417]: Undo APP-5417 (#536) [APP-5417]: Accommodate pagination for GetRobotPartHistory in api (#535) Data 2816/get training job logs (#534) DATA-2792 - Add URL to UpdateRegistryItemRequest, but make it optional (#532) RSDK-8228 Add config status to machine status API (#531) Add revision field to config (#530) [APP-5420] Add ListMachineFragments request and response (#525) [APP-5400]: Add GetFragmentHistory proto (#524) RSDK-7899 Add MachineStatus to RobotService (#527) RSDK-8083: add LogPatternConfig field to RobotConfig API (#526) ...
No description provided.