Repository navigation
air: Report the MLflow retry in air get -o json - #7620
Draft
artemy-kolesnikov wants to merge 2 commits into
Draft
artemy-kolesnikov wants to merge 2 commits into
artemy-kolesnikov wants to merge 2 commits into
Conversation
The retry fallback lands in mlflowIDsFromRetries, which sets OlderRetry when the MLflow identifiers come from an earlier retry (a last retry that failed before it started never got an MLflow run of its own). The text views show it as "(retry N)", but getData never output it, so `air get -o json` returned an earlier retry's MLflow IDs with no field saying which retry they came from. Add MLflowRetry (*int, json:"mlflow_retry,omitempty") to getData, set alongside mlflow_url. It is absent when the IDs come from the latest retry, so existing JSON output (and the acceptance goldens) is unchanged. Covered by a full-command JSON unit test for both shapes (older retry present with mlflow_retry: 0; latest attempt absent), and a new acceptance fixture pinning the no-MLflow-output envelope. Stacks on #7456. Co-authored-by: Isaac <no-reply@databricks.com>
Contributor
|
Commit: Regression test reportSelected 1 changed tests (limit=20, 0 not selected) 1 tests: regression=0 unreleased=0 coverage=1 failing=0
|
Collaborator
✅ Integration test reportCommit: 8777f4b
Top 3 slowest tests (at least 2 minutes):
|
Co-authored-by: Isaac <no-reply@databricks.com>
This branch has not been deployed
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
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.
Title: air: Report the MLflow retry in air get -o json
Changes
Since #7456, when a run's last retry failed before it started, the views link to an earlier retry's MLflow run. The text views say so ("(retry N)"), but
air get -o jsonreturned the earlier retry's MLflow IDs with no field saying which retry they came from. This change addsMLflowRetrytogetData, set alongsidemlflow_urlfrom the existingmlflowIdentifiers.OlderRetry:It is a
*intwithjson:"mlflow_retry,omitempty", so it is absent when the MLflow IDs come from the run's latest retry. Responses where the link comes from an earlier retry intentionally gainmlflow_retry; all other responses, and every existing acceptance golden, are unchanged.Why
The retry fallback in #7456 was reviewed for the text views only (the
OlderRetryfield and its "(retry N)" rendering), and the JSON view was missed.mlflowIDsFromRetriesalready computes the retry; this only surfaces it.Tests
cmd/air/get_test.go: a new full-command JSON test (TestGetRunMLflowRetryJSON) runsair get -o jsonagainst a stub server where the last retry (attempt 2) never reached MLflow while attempt 0 did. It asserts the envelope containsmlflow_urlpointing at attempt 0's MLflow run andmlflow_retry: 0. A control run whose only attempt logged to MLflow asserts the envelope hasmlflow_urland nomlflow_retrykey.acceptance/air/get-mlflow-retry(modelled onlogs-failed-retry): a failed run whose retries have no MLflow output at all, assertingair get -o jsonemits a clean envelope with neithermlflow_urlnormlflow_retry.air getgoldens are unchanged (verified by running the wholeacceptance/airsuite).go test ./cmd/air/...,./task fmt,./task lint(0 issues) and./task check-changelogpass.This pull request and its description were written by Isaac.