-
Notifications
You must be signed in to change notification settings - Fork 179
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
Add support for MLflow model manage #1058
Conversation
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
f6070ff
to
e29cb07
Compare
f35b6fa
to
b323e76
Compare
pkg/commands/model/create.go
Outdated
command.Flags().StringVar(&tagsStr, "tags", "", "model tags e.g. key1,key2=value2") | ||
command.Flags().StringVar(&versionDescription, "version-description", "", "model version description") | ||
command.Flags().StringVar(&versionTagsStr, "version-tags", "", "model version tags e.g. key1,key2=value2") | ||
command.Flags().StringVar(&versionSource, "version-source", "", "model version source") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
source is better than version-source.
pkg/commands/model/delete.go
Outdated
if mlflowTrackingUri == "" { | ||
return fmt.Errorf("MLflow tracking server URI must be specified by MLFLOW_TRACKING_URI environment variable") | ||
} | ||
if name == "" { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
use command.MarkFlagRequired
6f4ee9f
to
d328d32
Compare
Signed-off-by: Yi Chen <github@chenyicn.net>
d328d32
to
a4f2f24
Compare
/lgtm @ChenYi015 Thank you for your outstanding contribution! |
} | ||
} | ||
|
||
serving.PrintServingJob(job, mv, utils.TransferPrintFormat(format)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If there is no mlflow installed, what will be shown in CLI mode ?
} | ||
} | ||
|
||
training.PrintTrainingJob(job, mv, format, showEvent, showGPU) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what will show when running arena list
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/lgtm
/approve
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: ChenYi015, cheyang The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
What this PR does / why we need it:
Model management is an important aspect of the machine learning lifecycle, and currently, the Arena CLI tool lacks support for model management features. MLflow is an open source platform dedicated to managing machine learning lifecycles, with the MLflow Model Registry component acting as a centralized model store, providing a set of APIs, and a UI, to collaboratively manage the full lifecycle of an MLflow Model. It would be meaningful for Arena to support some form of model registry, and MLflow stands out as a good choice.
Which issue(s) this PR fixes:
Fixes #1059