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

Correctly use --profile flag passed for all bundle commands #571

Merged
merged 7 commits into from
Jul 12, 2023

Conversation

andrewnester
Copy link
Contributor

Changes

Correctly use --profile flag passed for all bundle commands.

Also adds a validation that if bundle configured host mismatches provided profile, it throws an error.

Tests

bundle.yml

bundle.yml

bundle:
  name: test

workspace:
  host: https://foo.com/

Case 1. Multiple matching profiles

andrew.nester@HFW9Y94129 wheel % databricks bundle validate
panic: resolve: https://foo.com: multiple profiles matched: DEFAULT-1, DEFAULT-2: please set DATABRICKS_CONFIG_PROFILE or provide --profile flag to specify one. Config: host=https://foo.com/, databricks_cli_path=../../cli/cli. Env: DATABRICKS_CLI_PATH

Case 2. Choosing profile with --profile flag

andrew.nester@HFW9Y94129 wheel % databricks bundle validate --profile DEFAULT-1
... <works correctly>

Case 3. Choosing profile with environment variable

andrew.nester@HFW9Y94129 wheel % DATABRICKS_CONFIG_PROFILE=DEFAULT-1 databricks bundle validate
... <works correctly>

Case 4. Flag has precedence over env variable

andrew.nester@HFW9Y94129 wheel % DATABRICKS_CONFIG_PROFILE=NOEXIST databricks bundle validate  --profile DEFAULT-1
... <works correctly>

bundle.yml

bundle:
  name: test

workspace:
  host: https://bar.com/
  
environments:
  development: {
    workspace: {
        host: https://baz.com/
    }
  }

  production: {
    workspace: {
        host: https://foo.com
    }
  }

Case 5. Profile host do not match bundle config host


andrew.nester@HFW9Y94129 wheel % databricks bundle validate  --profile DEFAULT-1
panic: config host mismatch, profile has https://foo.com host but CLI configured to use https://bar.com

Case 6. Profile host do not match bundle config host for chosen environment


andrew.nester@HFW9Y94129 wheel % databricks bundle validate  --profile DEFAULT-1 -e development
panic: config host mismatch, profile has https://foo.com host but CLI configured to use https://baz.com

Case 7. Profile host matches bundle config host for chosen environment


andrew.nester@HFW9Y94129 wheel % databricks bundle validate  --profile DEFAULT-1 -e production
... <works correctly>

@andrewnester
Copy link
Contributor Author

I'll follow up with some (unit) tests

Copy link
Contributor

@pietern pietern left a comment

Choose a reason for hiding this comment

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

Nice! This definitely needs a couple test cases. Approach LGTM.

libs/databrickscfg/ops.go Outdated Show resolved Hide resolved
libs/databrickscfg/ops.go Show resolved Hide resolved
bundle/config/workspace.go Outdated Show resolved Hide resolved
andrewnester and others added 2 commits July 11, 2023 17:18
Co-authored-by: Pieter Noordhuis <pieter.noordhuis@databricks.com>
Copy link
Contributor

@pietern pietern left a comment

Choose a reason for hiding this comment

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

LGTM

I would like to see a couple tests before merging. Especially for the validation function.

bundle/config/workspace.go Outdated Show resolved Hide resolved
@andrewnester andrewnester merged commit 650fb0e into main Jul 12, 2023
4 checks passed
@andrewnester andrewnester deleted the bundle-profile-flag branch July 12, 2023 12:09
@nfx nfx mentioned this pull request Jul 18, 2023
nfx added a commit that referenced this pull request Jul 18, 2023
* 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
* 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
* 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)).
pietern added a commit that referenced this pull request Oct 19, 2023
If a bundle configuration specifies a workspace host, and the user specifies a
profile to use, we perform a check to confirm that the workspace host in the
bundle configuration and the workspace host from the profile are identical. If
they are not, we return an error. The check was introduced in #571.

Previously, the code included an assumption that the client configuration was
already loaded from the environment prior to performing the check. This was not
the case, and as such if the user intended to use a non-default path to
`.databrickscfg`, this path was not used when performing the check.

The fix does the following:
* Resolve the configuration prior to performing the check.
* Don't treat the configuration file not existing as an error.
* Add unit tests.

Fixes #884.
github-merge-queue bot pushed a commit that referenced this pull request Oct 20, 2023
## Changes

If a bundle configuration specifies a workspace host, and the user
specifies a profile to use, we perform a check to confirm that the
workspace host in the bundle configuration and the workspace host from
the profile are identical. If they are not, we return an error. The
check was introduced in #571.

Previously, the code included an assumption that the client
configuration was already loaded from the environment prior to
performing the check. This was not the case, and as such if the user
intended to use a non-default path to `.databrickscfg`, this path was
not used when performing the check.

The fix does the following:
* Resolve the configuration prior to performing the check.
* Don't treat the configuration file not existing as an error.
* Add unit tests.

Fixes #884.

## Tests

Unit tests and manual confirmation.
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