APP-7615 Update list machine fragments to support versions #629
Conversation
proto/viam/app/v1/app.proto
Outdated
| @@ -976,13 +977,19 @@ message ListRobotsRequest { | |||
| string location_id = 1; | |||
| } | |||
|
|
|||
| message FragmentRequest { | |||
There was a problem hiding this comment.
quibble: I like your AdditionalFragment idea for the interface/field name symmetry
There was a problem hiding this comment.
Do you mean AdditionalFragment would be the name of this message? My thought here is that I don't like the word Request because it implies that its a message used at the top level of the API
There was a problem hiding this comment.
yeah, good point. i'm going to rename this to AdditionalFragment
proto/viam/app/v1/app.proto
Outdated
| message ListMachineFragmentsRequest { | ||
| // the machine_id used to filter fragments defined in a machine's parts. | ||
| // Also returns any fragments nested within the fragments defined in parts. | ||
| string machine_id = 1; | ||
| // additional fragment_ids to append to the response. useful when needing to view fragments that will be | ||
| // provisionally added to the machine alongside existing fragments. | ||
| repeated string additional_fragment_ids = 2; | ||
| repeated FragmentRequest additional_fragments = 3; |
There was a problem hiding this comment.
agreement: yup, I think adding the field is the right move. As discussed with ML, let's hold off on any sort of deprecated or reserved keyword for the old field - we can do the least to make sure the old field doesn't break until we can ensure its usage is removed from the app
ehhong
force-pushed
the
APP-7615/ehhong/list-machine-fragments
branch
from
8d41010 to
c9e4920
Compare
ehhong
force-pushed
the
APP-7615/ehhong/list-machine-fragments
branch
from
c9e4920 to
62288fe
Compare
| @@ -1046,13 +1047,20 @@ message ListRobotsRequest { | |||
| string location_id = 1; | |||
| } | |||
|
|
|||
| message AdditionalFragment { | |||
| string fragment_id = 1; | |||
| optional string version = 2; | |||
There was a problem hiding this comment.
i updated this to optional. does that seem right to everyone?
ehhong
force-pushed
the
APP-7615/ehhong/list-machine-fragments
branch
from
0c69cc4 to
c037a5f
Compare
ehhong
added
the
ready-for-protos
ehhong
removed
the
ready-for-protos
tech spec
Open Qs
repeated string additional_fragment_ids = 2;? or should we support both fields?