DATA-3438 Add resourceSubtype to data source specification #594
Conversation
kaywux
added
the
ready-for-protos
proto/viam/app/data/v1/data.proto
Outdated
| @@ -234,6 +234,7 @@ message GetLatestTabularDataRequest { | |||
| string part_id = 1; | |||
| string resource_name = 2; | |||
| string method_name = 3; | |||
| string resource_type = 4; | |||
There was a problem hiding this comment.
would it make more sense to call this resource_subtype? according to the viam glossary resource type is either component or service and subtype is the actual component/service type like sensor or arm etc.
There was a problem hiding this comment.
Good callout! Changing this to resource_subtype instead
proto/viam/app/data/v1/data.proto
Outdated
| @@ -234,6 +234,7 @@ message GetLatestTabularDataRequest { | |||
| string part_id = 1; | |||
| string resource_name = 2; | |||
| string method_name = 3; | |||
| string resource_type = 4; | |||
There was a problem hiding this comment.
Is there a reason that this is after method_name instead of before?
There was a problem hiding this comment.
Yes - this is to keep method_name with the field number 3 which it's already been defined with. For protobuf the number each field is assigned to can't really be changed once the message type is in use, so if I were to have resource_type "before" method_name here, it would require us to do something like this:
string resource_subtype = 4;
string method_name = 5;
reserved 3;
which seems unnecessary given that the field order here doesn't really matter semantically.
There was a problem hiding this comment.
Ooh, in my head it wasn't really in use yet. But I guess that's not true! 😬
kaywux
changed the title
kaywux
added
ready-for-protos
Currently a data source is defined as a unique <part_id, resource_name, method_name> tuple. We should also add resource_type to this specification.
Corresponding app change: https://github.com/viamrobotics/app/pull/6947