Repository navigation
air: Use the run index for a self user filter and bound the index fallback in air list - #7619
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
…lback in air list `air list --all-status --filter user=<own email>` skipped the AiTrainingService index because the current user was only resolved when no user filter was set, so it scanned the workspace's full Jobs run history (minutes on a busy workspace). Resolve the current user for --all-status and treat a case-insensitive match as self, so that filter uses the index. When the index load fails, warn instead of logging at debug, and cap the Jobs scan that replaces it at 250 runs (it can't use active_only, so it walks the full history). Give index requests a 15 s request timeout and retry window so a hung index fails fast instead of waiting 60 s per page. Warn after the interactive table exits when a scan hit its cap. Co-authored-by: Isaac <no-reply@databricks.com>
Contributor
|
Commit: Regression test reportSelected 1 changed tests (limit=20, 0 not selected) 1 tests: regression=1 unreleased=0 coverage=0 failing=0
TestAccept/air/list-index-fallback: branch ✅ | main (c4559ed) ❌ | latest (v1.26.0) ❌main (c4559ed): latest (v1.26.0): |
Collaborator
✅ Integration test reportCommit: ba33c5c
Top 3 slowest tests (at least 2 minutes):
|
…pdate the fallback golden 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: Use the run index for a self user filter and bound the index fallback in air list
Changes
--all-status --filter user=<own email>now uses the AiTrainingService index.air listresolved the current user only when nouser=filter was set. So filtering on your own email never matchedcurrentUser, and the command scanned the workspace's full Jobs run history. With--all-status, the current user is now resolved even when an explicituser=filter is given, and a case-insensitive match counts as self. If that lookup fails, the command keeps the old scan rather than failing. The default active view still makes no extra call.active_only, so it walks the full history. It is now capped atmaxFallbackListScan = 250runs instead ofmaxListScan = 2000. Hitting either cap warns "results may be incomplete. Narrow the search with --filter or a smaller --limit". The interactive table now gives the same warning when it exits; before, only one-shot output did.w.ConfigwithHTTPTimeout = 15sandRetryTimeout = 15s, following the dedicated-client pattern invalidateconfig.go. Before, they used the SDK defaults: a 60 s inactivity timeout and a 5 min retry window. The global SDK config is unchanged.-o jsonstdout is unchanged. All new messages go to stderr.Why
A report against a workspace with a large run history had
air listtake minutes to return a small number of rows. The traces showed each call walking every full-historyruns/listpage (several seconds per page), with noai-training/workflowsindex request and noscim/Mecall from the list path: the index was never attempted, because the user had passed--filter user=...or--all-users. A self filter is the case this PR fixes. Manually verified against a workspace: the same self-filtered--all-statuslist drops from minutes to a few seconds, with identical rows.Tests
cmd/air/list_index_test.go:TestListSelfUserFilterUsesIndexruns the full command (--all-status --filter user=me@…, JSON) against a stub server. It asserts one index call, zeroruns/listcalls, and the run in stdout. It fails onmain: 0 index calls, 1 scan, empty result.TestFallbackScanWarnsAndIsBounded: the index returns 500 andruns/listserves endless non-AIR pages. It asserts exactly 250/25 = 10 pages, noactive_only, both warnings logged, and empty stdout.TestRegularScanKeepsFullCap:--all-usersstill scans up to 2000 runs, with no fallback warning.TestNewListStrategyGate: two new cases, self filter in a different case → index, and unknown current user → scan.TestNewListStrategyFallsBackWhenIndexFailsnow uses a cmdio context. Its assertions are unchanged.acceptance/air/list-index-fallback: the index returns 500. The text output shows the Warn line, and the JSON output (stderr dropped) is unchanged. No existingacceptance/air/list*golden changed: the new line appears only when the index fails, and no existing fixture fails it.go test ./cmd/air/...passes.go test ./acceptance -run TestAccept/air/passes (21 tests).gofmtandgo vetare clean.Follow-ups (not in this PR)
--all-statuswalks the caller's whole index without a bound (63 pages for a heavy user, about 4 s). The index has no server-side ordering or limit.handleListTrainingWorkflowsalso ignoresactive_only, so the default active view can't use the index. Both need a server-side state filter plus an ordered, limited index.--all-users --all-statusis still a full-history scan, capped at 2000 runs. That is inherent to Jobsruns/list(several seconds per page on a busy workspace).airCLI parity was skipped: the legacy CLI is frozen.This pull request and its description were written by Isaac.