fix(run): add namespace id in response #511
Conversation
joremysh
requested review from
donch1989,
heiruwu,
pinglin and
xiaofei-du
as code owners
|
The latest Buf updates on your PR. Results from workflow Buf CI / buf (pull_request).
|
triggered by commit: 34c062a
| @@ -1773,6 +1773,8 @@ message ModelRun { | |||
| // Requester ID. This field might be empty if the model run belongs to a | |||
| // deleted namespace. | |||
| string requester_id = 16 [(google.api.field_behavior) = OUTPUT_ONLY]; | |||
| // Namespace ID. | |||
| string namespace_id = 17 [(google.api.field_behavior) = OUTPUT_ONLY]; | |||
There was a problem hiding this comment.
The difference between fields such as namespace, requester and runner ID are getting more and more confusing. This is the namespace of the model, right? Why not calling it model_namespace, if it sits next to model_id? Also, model_uid identifies uniquely a model, but if we need this I assume no client is using that field (which is right, in public APIs we should use the public ID). Can that field be removed?
Documenting the difference between runner and requester would be good, too. Every time I read this contract I need to think how they differ.
There was a problem hiding this comment.
@jvallesm FE won't know what is the namespace this resource belong to if we don't have this, another solution is using something like model_name and I will handle the interpretation of it
There was a problem hiding this comment.
@EiffelFly I understand the need for this field, my comment is addressed to the naming (namespace can be interpreted as related to the model, not the run). I also thought of model_name but since we refactored our contracts from there to namespace + id, I think having 2 fields for this is better.
There was a problem hiding this comment.
- remove model_uid
- rename namespace_id to model_namespace_id
- update model_namespace_id documentations
Because
This commit