Unhide auth logout and add traceability comments#4719
Unhide auth logout and add traceability comments#4719mihaimitrea-db wants to merge 4 commits intomainfrom
Conversation
The auth logout feature (PRs #4613, #4616, #4647) was squashed into a single commit cb3c326 titled after #4647 only, losing traceability for the new command itself. This followup: - Adds a comment block to logout.go linking each original PR with its commit range and purpose. - Adds a NEXT_CHANGELOG entry for auth logout under CLI. - Removes Hidden: true so the command appears in help and completion. - Aligns the Long description with the public documentation.
|
Commit: be532db
16 interesting tests: 7 KNOWN, 7 SKIP, 2 flaky
Top 27 slowest tests (at least 2 minutes):
|
simonfaltum
left a comment
There was a problem hiding this comment.
Review from OpenCode + Isaac (2-round swarm review).
Verdict: Approved with a few suggestions on the help text.
0 Critical | 0 Major | 0 Gap | 4 Nit | 1 Suggestion
cmd/auth/logout.go
Outdated
| Command behavior: | ||
|
|
||
| 1. If you specify --profile, the command logs out of that profile. In an | ||
| interactive terminal you'll be asked to confirm unless --force is set. | ||
|
|
||
| 2. If you omit --profile in an interactive terminal, you'll be shown | ||
| an interactive picker listing all profiles from your configuration file. | ||
| You can search by profile name, host, or account ID. After selecting a | ||
| profile, you'll be asked to confirm unless --force is specified. | ||
| interactive terminal, it asks for confirmation unless you also specify | ||
| --force. | ||
|
|
||
| 3. If you omit --profile in a non-interactive environment (e.g. CI/CD pipeline), | ||
| the command will fail with an error asking you to specify --profile. | ||
| 2. If you omit --profile in an interactive terminal, the command shows a | ||
| searchable profile picker. You can search by profile name, host, or | ||
| account ID. After you select a profile, the command asks for confirmation | ||
| unless you also specify --force. | ||
|
|
||
| 4. Use --force to skip the confirmation prompt. This is required when | ||
| running in non-interactive environments. | ||
| 3. If you omit --profile in a non-interactive environment, the command fails | ||
| and asks you to specify --profile. | ||
|
|
||
| 5. Use --delete to also remove the selected profile from ~/.databrickscfg.`, | ||
| 4. In a non-interactive environment, use --profile together with --force to | ||
| skip confirmation.`, |
There was a problem hiding this comment.
[Nit] The old description had 5 numbered items with a dedicated point for --delete. The new list covers --force (item 4) but drops --delete, which is only mentioned in the opening paragraph. This creates an asymmetry where --force gets its own item but --delete does not. Consider adding an item like: "Use --delete to also remove the profile from the configuration file."
Also: I believe we are introducing a global --yes flag in a separate PR. Once that lands, --force here should probably be replaced with --yes for consistency. Worth noting as a follow-up.
There was a problem hiding this comment.
Yes, I am waiting for the --yes PR to be merged. Then I will create a PR for that change to ensure we are not releasing this command while also making changes to it. After that is fine we can merge this one.
Clarify shared token cleanup and align the help text with the actual non-interactive and --delete behavior so the public description is less misleading.
Changes
cmd/auth/logout.golinking to the three original PRs (Add auth logout command with --profile and --force flags #4613, Add interactive profile picker to auth logout #4616, Extract shared SelectProfile helper to eliminate duplicate profile pickers #4647) and their commit ranges. These PRs were inadvertently squashed into a single commitcb3c326titled after Extract shared SelectProfile helper to eliminate duplicate profile pickers #4647 only.NEXT_CHANGELOG.mdentry forauth logoutunder the CLI section.Hidden: truefrom the logout command so it appears indatabricks auth -hand tab completion.databricks auth loginWhy
The
auth logoutfeature was implemented across three stacked PRs (#4613, #4616, #4647) that were squashed into one commit on merge. The commit title only references the last PR ("Extract shared SelectProfile helper"), soauth logoutitself has no changelog entry and no git-blame traceability.This followup restores that context, unhides the command for users, and adds the missing changelog entry.
Tests
No functional changes; existing unit and acceptance tests for
auth logoutcontinue to pass.