Add POST version of GetLatestVersions api#4999
Conversation
Signed-off-by: Steven Chen <s.chen@databricks.com>
Signed-off-by: Steven Chen <s.chen@databricks.com>
Signed-off-by: Steven Chen <s.chen@databricks.com>
Signed-off-by: Steven Chen <s.chen@databricks.com>
Signed-off-by: Steven Chen <s.chen@databricks.com>
| try: | ||
| return call_endpoint(host_creds, endpoint, method, json_body, response_proto) | ||
| except RestException as e: | ||
| if e.error_code != ErrorCode.Name(ENDPOINT_NOT_FOUND) or i == len(endpoints) - 1: |
There was a problem hiding this comment.
how do we guarantee that we try the POST endpoint before the GET endpoint for GetLatestVersions? why do we only check for ENDPOINT_NOT_FOUND here?
There was a problem hiding this comment.
The order that the request is made is defined by the order in the service proto, so I added in the POST endpoint before the GET. Yeah the ENDPOINT_NOT_FOUND is a good callout. The databricks backend returns ENDPOINT_NOT_FOUND, so I wanted to make sure only this exception would be allowed to pass to prevent this function from masking other exceptions. I'm not sure how other backends handle ENDPOINT_NOT_FOUND though, so maybe there's a better condition to check for. cc @sueann
There was a problem hiding this comment.
ooh i see, makes sense!
might be good to document/add a comment here about the ordering of endpoints since that detail is quite subtle
Signed-off-by: Steven Chen <s.chen@databricks.com>
Signed-off-by: Steven Chen <s.chen@databricks.com>
What changes are proposed in this pull request?
For some backends, when calling the GetLatestVersions api with a stages parameter, only the first stage would be used due to how GET requests handle lists of query params (
name=model&stages=production&stages=staging) . This PR adds a POST version of the GetLatestVersions api and a newcall_endpointsutil function that tries all endpoints for an API.How is this patch tested?
Release Notes
Is this a user-facing change?
(Details in 1-2 sentences. You can just refer to another PR with a description if this PR is part of a larger change.)
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