Support specifying 'latest' in model URI to get the latest version of a model regardless of the stage#5027
Merged
Merged
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>
ankit-db
reviewed
Dec 27, 2021
ankit-db
left a comment
Contributor
There was a problem hiding this comment.
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)) |
Contributor
There was a problem hiding this comment.
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?
Contributor
Author
There was a problem hiding this comment.
Actually model version is str: link. So it's safer to convert it into int here.
| @@ -46,12 +59,16 @@ def _parse_model_uri(uri): | |||
|
|
|||
| if parts[1].isdigit(): | |||
Contributor
There was a problem hiding this comment.
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
Signed-off-by: Chenran Li <chenran.li@databricks.com>
ankit-db
approved these changes
Jan 10, 2022
23 tasks
This was referenced Jan 26, 2022
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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>/latestto get the latest version of a model regardless of the stage.Also fix a bug in
sqlalchemy_store.get_latest_versions(): when nostageargument is specified, it should return the latest versions of all stages. Now it's only returning latest versions for active stages.stageargument is specifiedIn #4250, Anil also suggested supporting model URI formats like
models:/<model_name>/latest-nto get the nth to last model version. But it requires too big of a change (e.g. extending theRegisteredModelclass 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>/latestto 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