[Feature] Add support for Unified Host with experimental flag#4260
Conversation
|
Commit: be536d3
32 interesting tests: 14 flaky, 7 KNOWN, 5 SKIP, 4 RECOVERED, 2 BUG
Top 50 slowest tests (at least 2 minutes):
|
|
Please include relevant PRs on SDK side in the description. I found these databricks/databricks-sdk-go#1307 (depends on) databricks/databricks-sdk-go#1403 (not used yet, but should be?) |
|
Thanks for the review @denik, added the relevant PR in the description. |
Thanks @denik added. |
| func promptForWorkspaceID(ctx context.Context) (string, error) { | ||
| if !cmdio.IsPromptSupported(ctx) { | ||
| // Workspace ID is optional for unified hosts, so return empty string in non-interactive mode | ||
| return "", nil |
There was a problem hiding this comment.
Why is it optional? If I understand correctly, we cannot make API calls without a workspace ID.
There was a problem hiding this comment.
And then this should return an error.
There was a problem hiding this comment.
This is optional because without workspace ID we configure the client for account to make account requests.
There was a problem hiding this comment.
Does the caller error if neither an account ID nor a workspace ID is specified?
There was a problem hiding this comment.
Yes
Error: fetching oauth config: fetching OAuth endpoints: failed to get OAuth endpoints: received HTML response instead of JSON
There was a problem hiding this comment.
That's not actionable. I was thinking of a proper error saying that you need either an account ID or a workspace ID.
|
Commit: 5c0467c
29 interesting tests: 13 flaky, 7 KNOWN, 5 SKIP, 4 RECOVERED
Top 50 slowest tests (at least 2 minutes):
|
## Changes <!-- Brief summary of your changes that is easy to understand --> - Add support for unified host with experimental flag. - Prompt for workspace id as it's not part of the host - Write the IDs and flag in the created profile. - Depends on databricks/databricks-sdk-go#1307 ## Why <!-- Why are these changes needed? Provide the context that the reviewer might be missing. --> This support is required for enabling unified hosts which Databricks free edition uses. ## Tests <!-- How have you tested the changes? --> - Unit tests - Manual E2E tests -- CUJ for CLI U2M `databricks auth login --host <spog-host> --experimental-is-unified-host` prompts for account and workspace id followed by opening web browser. The IDs and flag is stores in the config. The subsequent cli operations eg: databricks clusters list --profile "above-profile" works.
Changes
Why
This support is required for enabling unified hosts which Databricks free edition uses.
Tests
databricks auth login --host <spog-host> --experimental-is-unified-hostprompts for account and workspace id followed by opening web browser. The IDs and flag is stores in the config. The subsequent cli operations eg: databricks clusters list --profile "above-profile" works.