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

Infer host from profile during login #629

Merged
merged 2 commits into from
Aug 2, 2023

Conversation

mgyucht
Copy link
Contributor

@mgyucht mgyucht commented Jul 31, 2023

Changes

A pretty annoying part of the current CLI experience is that when logging in with databricks auth login, you always need to type the name of the host. This seems unnecessary if you have already logged into a host before, since the CLI can read the previous host from your .databrickscfg file.

This change handles this case by setting the host if unspecified to the host in the corresponding profile. Combined with autocomplete, this makes the login process simple:

databricks auth login --profile prof<tab><enter>

Tests

Logged in to an existing profile by running the above command (but for a real profile I had).

@@ -43,8 +43,36 @@ func newLoginCommand(persistentAuth *auth.PersistentAuth) *cobra.Command {

cmd.RunE = func(cmd *cobra.Command, args []string) error {
ctx := cmd.Context()

var profileName string
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Profile prompt was used also to store new profile with a chosen name(or update existing one)
So for example, it would me allow to override "dogfood" profile with a new host even if I have already one.

With this change if I understand it correctly, it's not possible, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Users can update the host used for a profile by passing --host on the CLI, so this would remove the prompt for hostname if the profile exists. My assumption is that people typically do not change the hostname for a profile; 99% of the time, they are reauthing to a host that they've already configured.

} else {
prompt := cmdio.Prompt(ctx)
prompt.Label = "Databricks Profile Name"
prompt.Default = persistentAuth.ProfileName()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Previously it proposed default value based on host name, what does it prompts now?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be just empty string, as we don't know where they are trying to authenticate to.

@mgyucht
Copy link
Contributor Author

mgyucht commented Aug 1, 2023

BTW: I'm also open to leaving the prompt as is but populating the default value as the host from .databrickscfg, but I feel that is slightly inferior as it requires the user to hit enter a second time.

@mgyucht mgyucht added this pull request to the merge queue Aug 2, 2023
Merged via the queue into main with commit d9ab465 Aug 2, 2023
4 checks passed
@mgyucht mgyucht deleted the infer-host-from-profile-during-login branch August 2, 2023 09:51
pietern added a commit that referenced this pull request Aug 10, 2023
CLI:
 * Infer host from profile during `auth login` ([#629](#629)).

Bundles:
 * Extend deployment mode support ([#577](#577)).
 * Add validation for Git settings in bundles ([#578](#578)).
 * Only treat files with .tmpl extension as templates ([#594](#594)).
 * Add JSON schema validation for input template parameters ([#598](#598)).
 * Add DATABRICKS_BUNDLE_INCLUDE_PATHS to specify include paths through env vars ([#591](#591)).
 * Initialise a empty default bundle if BUNDLE_ROOT and DATABRICKS_BUNDLE_INCLUDES env vars are present ([#604](#604)).
 * Regenerate bundle resource structs from latest Terraform provider ([#633](#633)).
 * Fixed processing jobs libraries with remote path ([#638](#638)).
 * Add unit test for file name execution during rendering ([#640](#640)).
 * Add bundle init command and support for prompting user for input values ([#631](#631)).
 * Fix bundle git branch validation ([#645](#645)).

Internal:
 * Fix mkdir integration test on GCP ([#620](#620)).
 * Fix git clone integration test for non-existing repo ([#610](#610)).
 * Remove push to main trigger for build workflow ([#621](#621)).
 * Remove workflow to publish binaries to S3 ([#622](#622)).
 * Fix failing fs mkdir test on azure ([#627](#627)).
 * Print y/n options when displaying prompts using cmdio.Ask ([#650](#650)).

API Changes:
 * Changed `databricks account metastore-assignments create` command to not return anything.
 * Added `databricks account network-policy` command group.

OpenAPI commit 7b57ba3a53f4de3d049b6a24391fe5474212daf8 (2023-07-28)

Dependency updates:
 * Bump OpenAPI specification & Go SDK Version ([#624](#624)).
 * Bump golang.org/x/term from 0.10.0 to 0.11.0 ([#643](#643)).
 * Bump golang.org/x/text from 0.11.0 to 0.12.0 ([#642](#642)).
 * Bump golang.org/x/oauth2 from 0.10.0 to 0.11.0 ([#641](#641)).
@pietern pietern mentioned this pull request Aug 10, 2023
github-merge-queue bot pushed a commit that referenced this pull request Aug 10, 2023
CLI:
* Infer host from profile during `auth login`
([#629](#629)).

Bundles:
* Extend deployment mode support
([#577](#577)).
* Add validation for Git settings in bundles
([#578](#578)).
* Only treat files with .tmpl extension as templates
([#594](#594)).
* Add JSON schema validation for input template parameters
([#598](#598)).
* Add DATABRICKS_BUNDLE_INCLUDE_PATHS to specify include paths through
env vars ([#591](#591)).
* Initialise a empty default bundle if BUNDLE_ROOT and
DATABRICKS_BUNDLE_INCLUDES env vars are present
([#604](#604)).
* Regenerate bundle resource structs from latest Terraform provider
([#633](#633)).
* Fixed processing jobs libraries with remote path
([#638](#638)).
* Add unit test for file name execution during rendering
([#640](#640)).
* Add bundle init command and support for prompting user for input
values ([#631](#631)).
* Fix bundle git branch validation
([#645](#645)).

Internal:
* Fix mkdir integration test on GCP
([#620](#620)).
* Fix git clone integration test for non-existing repo
([#610](#610)).
* Remove push to main trigger for build workflow
([#621](#621)).
* Remove workflow to publish binaries to S3
([#622](#622)).
* Fix failing fs mkdir test on azure
([#627](#627)).
* Print y/n options when displaying prompts using cmdio.Ask
([#650](#650)).

API Changes:
* Changed `databricks account metastore-assignments create` command to
not return anything.
 * Added `databricks account network-policy` command group.

OpenAPI commit 7b57ba3a53f4de3d049b6a24391fe5474212daf8 (2023-07-28)

Dependency updates:
* Bump OpenAPI specification & Go SDK Version
([#624](#624)).
* Bump golang.org/x/term from 0.10.0 to 0.11.0
([#643](#643)).
* Bump golang.org/x/text from 0.11.0 to 0.12.0
([#642](#642)).
* Bump golang.org/x/oauth2 from 0.10.0 to 0.11.0
([#641](#641)).
github-merge-queue bot pushed a commit that referenced this pull request Aug 14, 2023
## Changes
#629 introduced a change to autopopulate the host from .databrickscfg if
the user is logging back into a host they were previously using. This
did not respect the DATABRICKS_CONFIG_FILE env variable, causing the
flow to stop working for users with no .databrickscfg file in their home
directory.

This PR refactors all config file loading to go through one interface,
`databrickscfg.GetDatabricksCfg()`, and an auxiliary
`databrickscfg.GetDatabricksCfgPath()` to get the configured file path.

Closes #655.

## Tests
```
$ databricks auth login --profile abc
Error: open /Users/miles/.databrickscfg: no such file or directory

$ ./cli auth login --profile abc
Error: cannot load Databricks config file: open /Users/miles/.databrickscfg: no such file or directory


$ DATABRICKS_CONFIG_FILE=~/.databrickscfg.bak ./cli auth login --profile abc
Databricks Host: https://asdf
```
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.

None yet

2 participants