Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Prompt once for a client profile #727

Merged
merged 5 commits into from
Sep 11, 2023
Merged

Prompt once for a client profile #727

merged 5 commits into from
Sep 11, 2023

Conversation

pietern
Copy link
Contributor

@pietern pietern commented Sep 4, 2023

Changes

The previous implementation ran the risk of infinite looping for the account client due to a mismatch in determining what constitutes an account client between the CLI and SDK (see here and here).

Ultimately, this code must never infinite loop. If a user is prompted and selects a profile that cannot be used, they should receive that feedback immediately and try again, instead of being prompted again.

Related to #726.

Tests

@pietern
Copy link
Contributor Author

pietern commented Sep 4, 2023

This cannot be merged until we also confirm that a request can be authenticated (we don't need to make one).

The previous implementation was prone to infinite looping for the account
client due to a mismatch in determining what constitutes an account client.

Ultimately, this code must never infinite loop. If a user is prompted and
selects a profile that cannot be used, they should receive that feedback
immediately and try again, instead of being prompted again.
* Authenticate empty request both for account and workspace client
* Prompt only on wrong or ambiguous client configuration
* Run selector through cmdio to pick up configured stdin/stderr
@pietern pietern marked this pull request as ready for review September 11, 2023 08:07
cmd/root/auth.go Outdated Show resolved Hide resolved
cmd/root/auth.go Outdated Show resolved Hide resolved
@pietern pietern enabled auto-merge September 11, 2023 15:21
@pietern pietern disabled auto-merge September 11, 2023 15:22
@pietern pietern enabled auto-merge September 11, 2023 15:27
@pietern pietern linked an issue Sep 11, 2023 that may be closed by this pull request
@pietern pietern added this pull request to the merge queue Sep 11, 2023
Merged via the queue into main with commit 0cb05d1 Sep 11, 2023
@pietern pietern deleted the prompt-auth-once branch September 11, 2023 15:43
pietern added a commit that referenced this pull request Sep 12, 2023
This release marks the public preview phase of Databricks Asset Bundles.

For more information, please refer to our online documentation at
https://docs.databricks.com/en/dev-tools/bundles/.

CLI:
 * Prompt once for a client profile ([#727](#727)).

Bundles:
 * Use clearer error message when no interpolation value is found. ([#764](#764)).
 * Use interactive prompt to select resource to run if not specified ([#762](#762)).
 * Add documentation link bundle command group description ([#770](#770)).
@pietern pietern mentioned this pull request Sep 12, 2023
github-merge-queue bot pushed a commit that referenced this pull request Sep 12, 2023
This release marks the public preview phase of Databricks Asset Bundles.

For more information, please refer to our online documentation at
https://docs.databricks.com/en/dev-tools/bundles/.

CLI:
* Prompt once for a client profile
([#727](#727)).

Bundles:
* Use clearer error message when no interpolation value is found.
([#764](#764)).
* Use interactive prompt to select resource to run if not specified
([#762](#762)).
* Add documentation link bundle command group description
([#770](#770)).
hectorcast-db pushed a commit that referenced this pull request Oct 13, 2023
## Changes

The previous implementation ran the risk of infinite looping for the
account client due to a mismatch in determining what constitutes an
account client between the CLI and SDK (see
[here](https://github.com/databricks/cli/blob/83443bae8d8ad4df3758f4192c6bbe613faae9c4/libs/databrickscfg/profiles.go#L61)
and
[here](https://github.com/databricks/databricks-sdk-go/blob/0fdc5165e57a4e7af6ec97b47595c6dddf37b10b/config/config.go#L160)).

Ultimately, this code must never infinite loop. If a user is prompted
and selects a profile that cannot be used, they should receive that
feedback immediately and try again, instead of being prompted again.

Related to #726.

## Tests
<!-- How is this tested? -->
hectorcast-db pushed a commit that referenced this pull request Oct 13, 2023
This release marks the public preview phase of Databricks Asset Bundles.

For more information, please refer to our online documentation at
https://docs.databricks.com/en/dev-tools/bundles/.

CLI:
* Prompt once for a client profile
([#727](#727)).

Bundles:
* Use clearer error message when no interpolation value is found.
([#764](#764)).
* Use interactive prompt to select resource to run if not specified
([#762](#762)).
* Add documentation link bundle command group description
([#770](#770)).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Login loop when using databricks account with azure service principle
3 participants