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.

Sign up for GitHub

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

Jump to bottom

Improve auth login experience #570

Merged
merged 7 commits into from
Jul 12, 2023
Merged

Improve auth login experience #570

merged 7 commits into from
Jul 12, 2023

Conversation

mgyucht
Copy link
Contributor

@mgyucht mgyucht commented Jul 11, 2023

Changes

Currently, databricks auth login is difficult to use. If a user types this command in, the command fails with

Error: init: cannot fetch credentials

after prompting for a profile name.

To make this experience smoother, this change ensures that the host, and if necessary, the account ID, are prompted for input from the user if they aren't provided on the CLI.

Tests

Manual tests:

$ ./cli auth token                   
Databricks Host: https://<HOST>.staging.cloud.databricks.com
{
  "access_token": "...",
  "token_type": "Bearer",
  "expiry": "2023-07-11T12:56:59.929671+02:00"
}
$ ./cli auth login
Databricks Host: https://<HOST>.staging.cloud.databricks.com
Databricks Profile Name: <HOST>-test
Profile <HOST>-test was successfully saved
$ ./cli auth login
Databricks Host: https://accounts.cloud.databricks.com
Databricks Account ID: <ACCOUNTID>
Databricks Profile Name: ACCOUNT-<ACCOUNTID>-test
Profile ACCOUNT-<ACCOUNTID>-test was successfully saved

andrewnester
andrewnester reviewed Jul 11, 2023
View reviewed changes
cmd/auth/auth.go Outdated
var perisistentAuth auth.PersistentAuth
var persistentAuth auth.PersistentAuth

func promptForHost(ctx context.Context) error {
Copy link
Contributor

Choose a reason for hiding this comment

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

I suggest to instead return (string, error) instead of setting a global variable in this function

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed

cmd/auth/auth.go Outdated
return nil
}

func promptForAccountId(ctx context.Context) error {
Copy link
Contributor

Choose a reason for hiding this comment

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

Same as above

Copy link
Contributor Author

@mgyucht mgyucht Jul 11, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Fixed

pietern
pietern approved these changes Jul 12, 2023
View reviewed changes
cmd/auth/auth.go Outdated Show resolved Hide resolved
mgyucht and others added 2 commits July 12, 2023 17:27
@mgyucht @pietern
Update cmd/auth/auth.go
2b8872d 
Co-authored-by: Pieter Noordhuis <pieter.noordhuis@databricks.com>
@mgyucht
Merge branch 'main' into better-auth-login
bea3452
@mgyucht mgyucht enabled auto-merge (squash) July 12, 2023 15:27
@mgyucht mgyucht merged commit 9a08881 into main Jul 12, 2023
4 checks passed
@mgyucht mgyucht deleted the better-auth-login branch July 12, 2023 16:13
@andrewnester andrewnester mentioned this pull request Jul 13, 2023
Closed
@nfx nfx mentioned this pull request Jul 18, 2023
Merged
nfx added a commit that referenced this pull request Jul 18, 2023
@nfx
Release v0.201.0
c2023a5 
* Add development runs ([#522](#522)).
* Support tab completion for profiles ([#572](#572)).
* Correctly use --profile flag passed for all bundle commands ([#571](#571)).
* Disallow notebooks in paths where files are expected ([#573](#573)).
* Improve auth login experience ([#570](#570)).
* Remove base path checks during sync ([#576](#576)).
* First look for databricks.yml before falling back to bundle.yml ([#580](#580)).
* Integrate with auto-release infra ([#581](#581)).

API Changes:

 * Removed `databricks metastores maintenance` command.
 * Added `databricks metastores enable-optimization` command.
 * Added `databricks tables update` command.
 * Changed `databricks account settings delete-personal-compute-setting` command with new required argument order.
 * Changed `databricks account settings read-personal-compute-setting` command with new required argument order.
 * Added `databricks clean-rooms` command group.

OpenAPI SHA: 850a075ed9758d21a6bc4409506b48c8b9f93ab4, Date: 2023-07-18
Dependency updates:

 * Bump golang.org/x/term from 0.9.0 to 0.10.0 ([#567](#567)).
 * Bump golang.org/x/oauth2 from 0.9.0 to 0.10.0 ([#566](#566)).
 * Bump golang.org/x/mod from 0.11.0 to 0.12.0 ([#568](#568)).
 * Bump github.com/databricks/databricks-sdk-go from 0.12.0 to 0.13.0 ([#585](#585)).
nfx added a commit that referenced this pull request Jul 18, 2023
@nfx
Release v0.201.0
000bd42 
* Add development runs ([#522](#522)).
* Support tab completion for profiles ([#572](#572)).
* Correctly use --profile flag passed for all bundle commands ([#571](#571)).
* Disallow notebooks in paths where files are expected ([#573](#573)).
* Improve auth login experience ([#570](#570)).
* Remove base path checks during sync ([#576](#576)).
* First look for databricks.yml before falling back to bundle.yml ([#580](#580)).
* Integrate with auto-release infra ([#581](#581)).

API Changes:

 * Removed `databricks metastores maintenance` command.
 * Added `databricks metastores enable-optimization` command.
 * Added `databricks tables update` command.
 * Changed `databricks account settings delete-personal-compute-setting` command with new required argument order.
 * Changed `databricks account settings read-personal-compute-setting` command with new required argument order.
 * Added `databricks clean-rooms` command group.

OpenAPI SHA: 850a075ed9758d21a6bc4409506b48c8b9f93ab4, Date: 2023-07-18
Dependency updates:

 * Bump golang.org/x/term from 0.9.0 to 0.10.0 ([#567](#567)).
 * Bump golang.org/x/oauth2 from 0.9.0 to 0.10.0 ([#566](#566)).
 * Bump golang.org/x/mod from 0.11.0 to 0.12.0 ([#568](#568)).
 * Bump github.com/databricks/databricks-sdk-go from 0.12.0 to 0.13.0 ([#585](#585)).
nfx added a commit that referenced this pull request Jul 18, 2023
@nfx
Release v0.201.0 (#586)
acf292d 
* Add development runs
([#522](#522)).
* Support tab completion for profiles
([#572](#572)).
* Correctly use --profile flag passed for all bundle commands
([#571](#571)).
* Disallow notebooks in paths where files are expected
([#573](#573)).
* Improve auth login experience
([#570](#570)).
* Remove base path checks during sync
([#576](#576)).
* First look for databricks.yml before falling back to bundle.yml
([#580](#580)).
* Integrate with auto-release infra
([#581](#581)).

API Changes:

 * Removed `databricks metastores maintenance` command.
 * Added `databricks metastores enable-optimization` command.
 * Added `databricks tables update` command.
* Changed `databricks account settings delete-personal-compute-setting`
command with new required argument order.
* Changed `databricks account settings read-personal-compute-setting`
command with new required argument order.
 * Added `databricks clean-rooms` command group.

OpenAPI SHA: 850a075ed9758d21a6bc4409506b48c8b9f93ab4, Date: 2023-07-18
Dependency updates:

* Bump golang.org/x/term from 0.9.0 to 0.10.0
([#567](#567)).
* Bump golang.org/x/oauth2 from 0.9.0 to 0.10.0
([#566](#566)).
* Bump golang.org/x/mod from 0.11.0 to 0.12.0
([#568](#568)).
* Bump github.com/databricks/databricks-sdk-go from 0.12.0 to 0.13.0
([#585](#585)).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@pietern pietern pietern approved these changes

@andrewnester andrewnester Awaiting requested review from andrewnester

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@mgyucht @pietern @andrewnester