Generalize CLI token source into progressive command list#1378
Generalize CLI token source into progressive command list#1378mihaimitrea-db wants to merge 2 commits intomainfrom
Conversation
bedfa97 to
f979a2c
Compare
Range-diff: stack/cli-force-refresh (bedfa97 -> f979a2c)
Reproduce locally: |
f979a2c to
b0cd110
Compare
Range-diff: stack/cli-force-refresh (f979a2c -> b0cd110)
Reproduce locally: |
b0cd110 to
bdc451f
Compare
Range-diff: stack/cli-force-refresh (b0cd110 -> bdc451f)
Reproduce locally: |
bdc451f to
8821700
Compare
Range-diff: stack/cli-force-refresh (bdc451f -> 8821700)
Reproduce locally: |
8821700 to
336b9be
Compare
Range-diff: stack/cli-force-refresh (8821700 -> 336b9be)
Reproduce locally: |
336b9be to
96fcf7f
Compare
96fcf7f to
b9159e8
Compare
Range-diff: stack/cli-force-refresh (96fcf7f -> b9159e8)
Reproduce locally: |
b9159e8 to
f8fff86
Compare
When the SDK's cached CLI token is stale, try `databricks auth token --force-refresh` to get a freshly minted token from the IdP. If the installed CLI is too old to recognise the flag, fall back to regular `auth token` and remember the capability for future refreshes. Centralise unknown-flag detection in CliTokenSource._exec_cli_command() via UnsupportedCliFlagError so the same classifier is reused by both the legacy --profile fallback and the new --force-refresh downgrade path in DatabricksCliTokenSource. See: databricks/cli#4767
f8fff86 to
29d8b73
Compare
Replace the explicit force_cmd/fallback_cmd fields with a CliCommand dataclass and an optional commands list on CliTokenSource. When commands is provided, refresh() delegates to _refresh_progressive() which walks the list from activeCommandIndex, falling back on unsupported-flag errors. When commands is None, refresh() delegates to _refresh_single() which preserves the original fallback behavior with zero changes for AzureCliTokenSource. DatabricksCliTokenSource._build_commands() produces the progressive list: --profile + --force-refresh first, plain --profile second, and --host as a terminal fallback. --force-refresh is only paired with --profile, never with --host. Adding future flags (e.g. --scopes) requires only adding entries to _build_commands().
29d8b73 to
1432f4c
Compare
Range-diff: stack/cli-force-refresh (29d8b73 -> 1432f4c)
Reproduce locally: |
|
If integration tests don't run automatically, an authorized user can run them manually by following the instructions below: Trigger: Inputs:
Checks will be approved automatically on success. |
🥞 Stacked PR
Use this link to review incremental changes.
Summary
Generalize
CliTokenSourcefrom the explicit_force_cmdfield and manual fallback override inDatabricksCliTokenSource.refresh()into aCliCommanddataclass and an optionalcommandslist onCliTokenSource, with an_active_command_indexthat caches which command works so subsequent token fetches skip probing.See: databricks/databricks-sdk-go#1605, databricks/databricks-sdk-java#752
Why
The parent PR (#1377) introduced
--force-refreshsupport by adding a_force_cmdfield toDatabricksCliTokenSourceand overridingrefresh()with hand-written fallback logic. This works, but every new flag would require adding another field, anothertry/exceptblock, another error check, and another set of tests — the pattern doesn't scale.We expect future flags like
--scopes(forwarding custom OAuth scopes to the CLI). Rather than growing the class linearly with each flag, this PR extracts the repeating pattern into a loop over a command list.Why try-and-retry over version detection or
--helpparsingThree approaches were evaluated for resolving which flags the installed CLI supports:
databricks version+ static version table) was rejected because it creates a maintenance burden and a second source of truth. Every SDK (Go, Python, Java) would need to independently maintain a table mapping flags to the CLI version that introduced them. If any SDK's table falls out of sync with the CLI's actual releases, users silently get degraded commands.--helpflag parsing (databricks auth token --help+ substring check) was rejected because it depends on the output format of--help— which is not a stable API. Cobra format changes could break detection, and naive substring matching is fragile.refresh()call, each command is tried in order; when the CLI responds with"unknown flag:", the next simpler command is tried. The working command index is cached so subsequent calls skip probing entirely. This approach has zero maintenance burden (no version numbers or flag registries to keep in sync), zero overhead on the happy path (newest CLI succeeds on the first command), and requires no signature changes. The key insight that makes this practical is that CLI flags are introduced incrementally — if the CLI doesn't support--profile, it certainly doesn't support--force-refresh.What changed
Interface changes
None.
CliTokenSourceis not part of the public API surface.Behavioral changes
None. The set of commands tried is identical to the parent PR. The only observable difference is that
_active_command_indexcaches the working command, so subsequentrefresh()calls execute that command directly without re-probing — a pure performance improvement.AzureCliTokenSourceis completely unchanged; it does not passcommandsand uses_refresh_single()which is an exact copy of the originalrefresh()logic.Internal changes
CliCommanddataclass: replaces the_force_cmdfield from the parent PR. Each entry holdsargs(the full CLI command),flags(used for error matching), andwarning(logged when falling back from this command).CliTokenSource.__init__: gains an optionalcommands: Optional[List[CliCommand]]parameter and initializes_active_command_index = -1(unresolved).CliTokenSource._is_unknown_flag_error: new static method that checks whether anIOErrormatches one of the given flags, preventing false-positive fallbacks.CliTokenSource.refresh(): now a delegating method — calls_refresh_progressive()whencommandsis set, otherwise calls_refresh_single()._refresh_single(): exact copy of the originalCliTokenSource.refresh()logic (cmd→fallback_cmdon"unknown flag: --profile"). Preserves full backward compatibility forAzureCliTokenSourceand any other caller not using the progressive chain._refresh_progressive(): checks_active_command_index— if resolved (≥ 0), calls the cached command directly. Otherwise delegates to_probe_and_exec()._probe_and_exec(): walkscommandsfrom index 0, falls back on unknown flag errors, stores_active_command_indexon success.DatabricksCliTokenSource.__init__: calls_build_commands()and passes the result tosuper().__init__(commands=...). Removes the_force_cmdfield from the parent PR.DatabricksCliTokenSource._build_commands(): static method that constructs the orderedList[CliCommand]—--profile+--force-refreshfirst, plain--profilesecond,--hostas a terminal fallback.--force-refreshis only paired with--profile, never with--host. Adding a future flag means adding one moreCliCommandliteral here.DatabricksCliTokenSource.refresh(): now a one-liner that callssuper().refresh()and validates token scopes.How is this tested?
Unit tests in
tests/test_credentials_provider.py:TestDatabricksCliTokenSourceArgs:test_profile_with_host_builds_three_commands— verifies 3CliCommandentries with correct args for profile+host config.test_profile_without_host_builds_two_commands— verifies 2 commands (no host fallback).test_host_only_builds_one_command— verifies single--hostcommand, no--force-refresh.test_account_client_passes_account_id— verifies--account-idis appended for account hosts.TestDatabricksCliForceRefresh:test_force_refresh_tried_first_with_profile— force command succeeds, no further commands tried.test_host_only_skips_force_refresh— host-only config does not attempt--force-refresh.test_force_refresh_fallback_when_unsupported— force command fails with"unknown flag: --force-refresh", falls back to plain--profile.test_profile_fallback_to_host— both force and profile fail, falls back to--host.test_full_fallback_chain— all three commands tried, last one succeeds.test_active_command_index_caching— first call probes and caches index; second call uses cached index directly (1 subprocess call instead of 2).test_real_auth_error_does_not_trigger_fallback— non-flag error surfaces immediately.test_is_unknown_flag_error— unit test for the static helper.