Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(model): move name and version out of updated model info message struct #29

Merged
merged 2 commits into from
Feb 24, 2022

Conversation

Phelan164
Copy link
Contributor

@Phelan164 Phelan164 commented Feb 24, 2022

Because

  • Model name and version is not allowed to update

This commit

  • Move model name and version out of updated model info message struct

@bochengyang
Copy link
Contributor

@Phelan164 I would like to suggest another way of the design of protobuf like this

rpc UpdatePipeline (UpdatePipelineRequest) returns (PipelineInfo) {
  option (google.api.http) = {
    patch: "/pipelines/{pipeline.name}"
    body: "pipeline"
  };
}

@Phelan164
Copy link
Contributor Author

@bochengyang I agree that we could use ModelInfo for updating. But there is some fields in ModelInfo do not allow to be updated. So I create an other message struct for updating model info.

Copy link
Contributor

@bochengyang bochengyang left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 🚀

@bochengyang bochengyang merged commit 919a751 into main Feb 24, 2022
@bochengyang bochengyang deleted the update-modelinfo branch February 24, 2022 08:50
@Phelan164
Copy link
Contributor Author

@bochengyang I got your point. It is elegant way. I updated following that

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Archived in project
Development

Successfully merging this pull request may close these issues.

2 participants