DATA rename list pipeline runs #675
Conversation
vijayvuyyuru
added
the
ready-for-protos
vijayvuyyuru
removed
the
ready-for-protos
|
I understand this feature isn't used by anyone in the public right now, but I'm not seeing the utility of this. If the scope was passed with the existing name, and that decision doesn't impact functionality, I feel we should live with our decision. We have lint rules about breaking backwards compatibility for a reason. And I prefer to not:
cc @ohEmily as the stakeholder that needs to ok this. |
| // ListPipelineRuns lists the runs of a data pipeline. | ||
| rpc ListPipelineRuns(ListPipelineRunsRequest) returns (ListPipelineRunsResponse); | ||
| // ListDataPipelineRuns lists the runs of a data pipeline. | ||
| rpc ListDataPipelineRuns(ListDataPipelineRunsRequest) returns (ListDataPipelineRunsResponse); |
There was a problem hiding this comment.
Also we're in the datapipelines package -- the stylistically correct name would be ListRuns.
Using datapipelines.ListDataPipelineRuns is inferior to datapipelines.ListRuns.
There was a problem hiding this comment.
this one is true for Go but not necessarily for gRPC -- in fact the proto package name here is viam.app.datapipelines.v1 and the AIP examples are all ListBooks GetBook etc: https://google.aip.dev/132
it's also the existing pattern in Viam APIs: https://github.com/viamrobotics/api/blob/main/proto/viam/app/dataset/v1/dataset.proto https://github.com/viamrobotics/api/blob/main/proto/viam/app/mltraining/v1/ml_training.proto
There was a problem hiding this comment.
Strictly speaking, yes. But almost all of the client and server architecture is Go. It's also a preferred style in many other languages (e.g: python and Java) that are simply too dated to have an official style guide on the relationship between package and type names.
This renames the listPipelineRuns endpoint to listDataPipelineRuns. All other parts also have data added to their name.