-
Notifications
You must be signed in to change notification settings - Fork 4.4k
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
Support specifying 'latest' in model URI to get the latest version of a model regardless of the stage #5027
Conversation
… a model regardless of the stage Signed-off-by: Chenran Li <chenran.li@databricks.com>
Signed-off-by: Chenran Li <chenran.li@databricks.com>
Signed-off-by: Chenran Li <chenran.li@databricks.com>
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.
The change looks good to me - just a few questions! I do think it may qualify as a breaking change though, so we should make sure to change the label
) | ||
return latest[0].version | ||
return max(map(lambda x: int(x.version), latest)) |
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.
Just curious - why do we need to call int()
here? I'm not opposed just for safety reasons, but x.version
is already a number right?
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.
Actually model version is str: link. So it's safer to convert it into int here.
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.
Got it
@@ -46,12 +59,16 @@ def _parse_model_uri(uri): | |||
|
|||
if parts[1].isdigit(): |
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.
Given the added complexity, it may be good to have an example of each URI type in the branch so that it's clear exactly which case maps to which tuple
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.
Done, thanks!
Signed-off-by: Chenran Li <chenran.li@databricks.com>
Signed-off-by: Chenran Li chenran.li@databricks.com
What changes are proposed in this pull request?
Support specifying 'latest' in model URI like
models:/<model_name>/latest
to get the latest version of a model regardless of the stage.Also fix a bug in
sqlalchemy_store.get_latest_versions()
: when nostage
argument is specified, it should return the latest versions of all stages. Now it's only returning latest versions for active stages.stage
argument is specifiedIn #4250, Anil also suggested supporting model URI formats like
models:/<model_name>/latest-n
to get the nth to last model version. But it requires too big of a change (e.g. extending theRegisteredModel
class and the corresponding proto message to store not only the latest versions of a model, but also all versions). It may not be worth it to make such a big change for this small "latest-n" feature. So I'm not doing this in this PR.How is this patch tested?
unit tests
Release Notes
Is this a user-facing change?
Now users can specify 'latest' in model URI like
models:/<model_name>/latest
to get the latest version of a model regardless of the stage. Previously users can only specifymodels:/<model_name>/<Stage>
to get the latest version of a model on a specific stage.What component(s), interfaces, languages, and integrations does this PR affect?
Components
area/artifacts
: Artifact stores and artifact loggingarea/build
: Build and test infrastructure for MLflowarea/docs
: MLflow documentation pagesarea/examples
: Example codearea/model-registry
: Model Registry service, APIs, and the fluent client calls for Model Registryarea/models
: MLmodel format, model serialization/deserialization, flavorsarea/projects
: MLproject format, project running backendsarea/scoring
: MLflow Model server, model deployment tools, Spark UDFsarea/server-infra
: MLflow Tracking server backendarea/tracking
: Tracking Service, tracking client APIs, autologgingInterface
area/uiux
: Front-end, user experience, plotting, JavaScript, JavaScript dev serverarea/docker
: Docker use across MLflow's components, such as MLflow Projects and MLflow Modelsarea/sqlalchemy
: Use of SQLAlchemy in the Tracking Service or Model Registryarea/windows
: Windows supportLanguage
language/r
: R APIs and clientslanguage/java
: Java APIs and clientslanguage/new
: Proposals for new client languagesIntegrations
integrations/azure
: Azure and Azure ML integrationsintegrations/sagemaker
: SageMaker integrationsintegrations/databricks
: Databricks integrationsHow should the PR be classified in the release notes? Choose one:
rn/breaking-change
- The PR will be mentioned in the "Breaking Changes" sectionrn/none
- No description will be included. The PR will be mentioned only by the PR number in the "Small Bugfixes and Documentation Updates" sectionrn/feature
- A new user-facing feature worth mentioning in the release notesrn/bug-fix
- A user-facing bug fix worth mentioning in the release notesrn/documentation
- A user-facing documentation change worth mentioning in the release notes