Interactive profile selection on multiple host matches#4604
Interactive profile selection on multiple host matches#4604simonfaltum wants to merge 5 commits intomainfrom
Conversation
When multiple profiles in ~/.databrickscfg match the same host, instead of erroring, the CLI now: filters to workspace-compatible profiles, auto-selects if only one remains, prompts interactively if multiple remain, or returns an actionable error in non-interactive mode.
- Use cmd.CommandPath() instead of hardcoded "databricks bundle deploy" in the non-interactive fix hint, so guidance matches the actual command - Add interactive prompt test that verifies the select prompt fires when PromptSupported is true and multiple workspace profiles match - Improve RetryWorkspaceClient test to prove re-execution by changing the profile between attempts and asserting the error message changes
simonfaltum
temporarily deployed
to
test-trigger-is
GitHub Actions
Inactive
|
Commit: d9ecb10
15 interesting tests: 7 KNOWN, 7 SKIP, 1 RECOVERED
Top 20 slowest tests (at least 2 minutes):
|
simonfaltum
temporarily deployed
to
test-trigger-is
GitHub Actions
Inactive
simonfaltum
requested review from
andrewnester,
anton-107,
denik,
pietern and
shreyas-goenka
as code owners
bundle/bundle.go
Outdated
| // WorkspaceClientE() to attempt client creation again on the next call. | ||
| // This is used after resolving profile ambiguity to retry with the | ||
| // selected profile. | ||
| func (b *Bundle) RetryWorkspaceClient() { |
There was a problem hiding this comment.
Maybe a better name is ClearWorkspaceClient?
There was a problem hiding this comment.
Makes sense, ClearWorkspaceClient is more accurate since it just clears the cache. Renamed.
|
|
||
| // promptForProfileByHost prompts the user to select a profile when multiple | ||
| // profiles match the same host. | ||
| func promptForProfileByHost(ctx context.Context, profiles profile.Profiles, host string) (string, error) { |
There was a problem hiding this comment.
Can you please add a screenshot / video what the selection UI looks like to the PR description?
There was a problem hiding this comment.
Will add a screenshot once I have an environment with two workspace profiles set up for the same host. The prompt shows a searchable list with profile names and their account/workspace IDs.
| logdiag.LogError(ctx, err) | ||
| return | ||
| // Check if this is a multi-profile ambiguity error. | ||
| names, isMulti := databrickscfg.AsMultipleProfiles(err) |
There was a problem hiding this comment.
style: can you refactor this to a separate method?
There was a problem hiding this comment.
Agreed, extracted to a dedicated function. Much cleaner.
cmd/root/bundle.go
Outdated
| case 1: | ||
| // Exactly one workspace-compatible profile — auto-select. | ||
| // This is deterministic and works in non-interactive mode. | ||
| selected = profiles[0].Name |
There was a problem hiding this comment.
Do we hit this line of code? I would assume this error branch would never trigger if the profile was successfully determined.
There was a problem hiding this comment.
Yes I think we do though it might not be happening until we live in a host-agnostic world. The SDK's ResolveProfileFromHost returns all profiles matching the host including account-only profiles. After we filter to workspace-compatible ones, we may end up with just one. The test TestBundleConfigureMultiMatchAutoSelectSingleWorkspace covers this: two profiles for the same host, one workspace and one account-only, and the workspace one is auto-selected.
cmd/root/bundle_test.go
Outdated
| cmd := emptyCommand(t) | ||
| ctx := logdiag.InitContext(cmd.Context()) | ||
| logdiag.SetCollect(ctx, true) | ||
| cmd.SetContext(ctx) |
There was a problem hiding this comment.
Lets convert any tests that run commands non-interactively to acceptance tests? They are often a better framework when testing CLI commands end to end.
There was a problem hiding this comment.
Done, converted the non-interactive tests to acceptance tests under acceptance/bundle/multi_profile/. Kept the interactive prompt test as a unit test since acceptance tests can't drive interactive prompts.
bundle/bundle.go
Outdated
| // WorkspaceClientE() to attempt client creation again on the next call. | ||
| // This is used after resolving profile ambiguity to retry with the | ||
| // selected profile. | ||
| func (b *Bundle) RetryWorkspaceClient() { |
There was a problem hiding this comment.
It might be better to encapsulate the prompt in the WorkspaceClientE method rather than clear the client and then set the client again. WDYT?
There was a problem hiding this comment.
I considered this but WorkspaceClientE lives in bundle/ which doesn't (and shouldn't) know about prompts or cmdio. The prompt depends on cobra. Command context and cmdio.RunSelect which are cmd-layer concerns.
- Rename RetryWorkspaceClient to ClearWorkspaceClient - Extract multi-profile handling into resolveProfileAmbiguity - Convert 4 non-interactive unit tests to acceptance tests - Keep interactive prompt test as unit test
simonfaltum
temporarily deployed
to
test-trigger-is
GitHub Actions
Inactive
3 participants
Why
When you have two profiles in
~/.databrickscfgthat point to the same workspace (e.g.devandstagingboth withhost = https://myworkspace.cloud.databricks.com), and you run a bundle command from a bundle that setsworkspace.hostbut notprofile, the CLI crashes with a confusing error telling you to setDATABRICKS_CONFIG_PROFILEor--profile.With the host-agnostic auth work, users are encouraged to have multiple profiles pointing at the same workspace (different auth methods, different roles). This "multiple profiles matched" error will become much more common. Today it's a paper cut; soon it'll be a wall.
Changes
Before: The CLI errored immediately with
multiple profiles matched: dev, staging: please set DATABRICKS_CONFIG_PROFILE or provide --profile flag. No way to recover without restarting the command with a flag or env var.Now: When the CLI detects multiple profiles matching the same host, instead of erroring it:
--profileflag, env var)Implementation is catch-and-retry in
configureBundle: letWorkspaceClientE()run normally, catcherrMultipleProfiles, resolve the ambiguity, set the profile on the workspace config, reset thesync.Oncecache viaRetryWorkspaceClient(), and retry. On retry the SDK uses named-profile resolution (sinceProfileis now set), which is the correct code path.New helpers:
AsMultipleProfiles(err)— extracts profile names from the error chainMatchProfileNames(names...)— filters profiles by nameRetryWorkspaceClient()— resets the workspace client cache for retrypromptForProfileByHost()— interactive select UI for profile disambiguationScope is bundle-only.
ResolveProfileFromHostis only wired inWorkspace.Client(), so non-bundle commands are unaffected.Test plan
AsMultipleProfilesextracts names through the full wrapping chain; returns false for unrelated/nil errorsMatchProfileNamesmatches correct profiles, rejects others, handles empty inputRetryWorkspaceClientproves re-execution by changing config between attempts and asserting the error message changesSkipPromptrespected: error returned, no promptPromptSupported: true→ select prompt starts on stderrDATABRICKS_TOKENset →errMultipleProfilesnever firesmake checks,make lintfullpass