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

Fix databricks configure to use DATABRICKS_CONFIG_FILE environment variable if exists as config file #1325

Merged

Conversation

kai-zhu-sonatype
Copy link
Contributor

Changes

added ConfigFile: cfg.ConfigFile for databrickscfg.SaveToProfile in cmd/configure/configure.go to save the file in a specified path when the value is not empty

Tests

TestConfigFileFromEnvNoInteractive in cmd/configure/configure_test.go sets a different config file path by DATABRICKS_CONFIG_FILE, after execution, the overwrite config file is generated, and the default path has no file.

@andrewnester
Copy link
Contributor

@kai-zhu-sonatype thanks for the contribution! could you please clarify what are you trying to achieve with this change?
current behaviour is if DATABRICKS_CONFIG_FILE is provided, cfg.ConfigFile should be set to this value as well and databrickscfg.SaveToProfile should save to DATABRICKS_CONFIG_FILE which effectively means that the pass of the file is going to be written in the profile.

@kai-zhu-sonatype
Copy link
Contributor Author

kai-zhu-sonatype commented Mar 28, 2024

Thanks @andrewnester for your response.
currently trying to use databrick configure with DATABRICKS_CONFIG_FILE, it seems not used to change config file location.
if looking the test TestConfigFileFromEnvNoInteractive in cmd/configure/configure_test.go, because the setup(t) is set HOME to tempHomeDir, then cfgPath still is the default path {HOME}/.databrickscfg, the test hasn't covered the case if the config file set a different path, for example: changing .databrickscfg to any other name .databrickscfg-test, the test will fail.

cfgPath := filepath.Join(tempHomeDir, ".databrickscfg-test")
t.Setenv("DATABRICKS_CONFIG_FILE", cfgPath)

adding to pass cfg.ConfigFile to databrickscfg.SaveToProfile, DATABRICKS_CONFIG_FILE is used.
debug log shows before and after the change
before: should save in /tmp/.databrickscfg-test, but still /home/ubuntu/.databrickscfg

❯ DATABRICKS_CONFIG_FILE=/tmp/.databrickscfg-test DATABRICKS_HOST=https://test DATABRICKS_TOKEN=TEST databricks configure --profile jenkins --debug
10:36:05  INFO start pid=7725 version=0.216.0 args="databricks, configure, --profile, jenkins, --debug"tabricks configure --profile jenkins --debug
10:36:05  INFO Backing up in /home/ubuntu/.databrickscfg.bak pid=7725
10:36:05  INFO Overwriting /home/ubuntu/.databrickscfg pid=7725
10:36:05  INFO completed execution pid=7725 exit_code=0

after: change to the specified location /tmp/.databrickscfg-test

❯ DATABRICKS_CONFIG_FILE=/tmp/.databrickscfg-test DATABRICKS_HOST=https://test DATABRICKS_TOKEN=TEST ./cli configure --profile jenkins --debug
10:35:31  INFO start pid=7675 version=0.0.0-dev+b0529cdd16c7 args="./cli, configure, --profile, jenkins, --debug"ure --profile jenkins --debug 
10:35:31  INFO Saving /tmp/.databrickscfg-test pid=7675
10:35:31  INFO completed execution pid=7675 exit_code=0

@andrewnester andrewnester requested a review from pietern March 28, 2024 15:08
@kai-zhu-sonatype kai-zhu-sonatype force-pushed the configure_use_databricks_config_file branch from 8d57f0c to 4886b9b Compare March 28, 2024 15:36
@kai-zhu-sonatype kai-zhu-sonatype force-pushed the configure_use_databricks_config_file branch from 4886b9b to 1f2dcd8 Compare March 28, 2024 15:51
@kai-zhu-sonatype kai-zhu-sonatype changed the title configure use DATABRICKS_CONFIG_FILE environment variable if exists as config file fix databricks configure to use DATABRICKS_CONFIG_FILE environment variable if exists as config file Mar 30, 2024
@kai-zhu-sonatype kai-zhu-sonatype changed the title fix databricks configure to use DATABRICKS_CONFIG_FILE environment variable if exists as config file Fix databricks configure to use DATABRICKS_CONFIG_FILE environment variable if exists as config file Mar 30, 2024
@codecov-commenter
Copy link

codecov-commenter commented Apr 2, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 53.77%. Comparing base (e22dd8a) to head (278c2e7).
Report is 160 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1325      +/-   ##
==========================================
+ Coverage   52.25%   53.77%   +1.52%     
==========================================
  Files         317      353      +36     
  Lines       18004    20480    +2476     
==========================================
+ Hits         9408    11014    +1606     
- Misses       7903     8661     +758     
- Partials      693      805     +112     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@kai-zhu-sonatype
Copy link
Contributor Author

kai-zhu-sonatype commented Apr 12, 2024

Hi @pietern, when you have time please have a look if this is the right way to fix databricks configure currently does not use DATABRICKS_CONFIG_FILE, the fix will help us to run cli on Jenkins easier. currently, we change HOME as a workaround.
Thanks

@pietern
Copy link
Contributor

pietern commented Apr 18, 2024

@kai-zhu-sonatype Thanks for the contribution!

Because of licensing reasons we're required to get a CLA signed before we can merge it. We don't have a smooth process for this setup quite yet, but I'll send an email to see if this is possible out of band.

Thanks for your patience.

@andrewnester
Copy link
Contributor

Fixes #1473

@andrewnester andrewnester enabled auto-merge June 17, 2024 10:06
@andrewnester andrewnester disabled auto-merge June 17, 2024 10:06
@andrewnester
Copy link
Contributor

@kai-zhu-sonatype could you please update the PR to latest base branch? It will retrigger the test as well and if everything passes we're ready to merge it

@kai-zhu-sonatype
Copy link
Contributor Author

Thanks @andrewnester for your approval, have merged the latest main to the branch

@andrewnester andrewnester enabled auto-merge June 24, 2024 10:49
@andrewnester andrewnester added this pull request to the merge queue Jun 24, 2024
Merged via the queue into databricks:main with commit 2ec6abf Jun 24, 2024
5 checks passed
pietern added a commit that referenced this pull request Jun 26, 2024
CLI:
 * Add link to documentation for Homebrew installation to README ([#1505](#1505)).
 * Fix `databricks configure` to use `DATABRICKS_CONFIG_FILE` environment variable if exists as config file ([#1325](#1325)).

Bundles:

The Terraform upgrade to v1.48.0 includes a fix for library order not being respected.

 * Fix conditional in query in `default-sql` template ([#1479](#1479)).
 * Remove user credentials specified in the Git origin URL ([#1494](#1494)).
 * Serialize dynamic value for `bundle validate` output ([#1499](#1499)).
 * Override variables with lookup value even if values has default value set ([#1504](#1504)).
 * Pause quality monitors when "mode: development" is used ([#1481](#1481)).
 * Return `fs.ModeDir` for Git folders in the workspace ([#1521](#1521)).
 * Upgrade TF provider to 1.48.0 ([#1527](#1527)).
 * Added support for complex variables ([#1467](#1467)).

Internal:
 * Add randIntn function ([#1475](#1475)).
 * Avoid multiple file tree traversals on bundle deploy ([#1493](#1493)).
 * Clean up unused code ([#1502](#1502)).
 * Use `dyn.InvalidValue` to indicate absence ([#1507](#1507)).
 * Add ApplyPythonMutator ([#1430](#1430)).
 * Set bool pointer to disable lock ([#1516](#1516)).
 * Allow the any type to be set to nil in `convert.FromTyped` ([#1518](#1518)).
 * Properly deal with nil values in `convert.FromTyped` ([#1511](#1511)).
 * Return `dyn.InvalidValue` instead of `dyn.NilValue` when errors happen ([#1514](#1514)).
 * PythonMutator: replace stdin/stdout with files ([#1512](#1512)).
 * Add context type and value to path rewriting ([#1525](#1525)).

API Changes:
 * Added schedule CRUD commands to `databricks lakeview`.
 * Added subscription CRUD commands to `databricks lakeview`.
 * Added `databricks apps start` command.

OpenAPI commit 7437dabb9dadee402c1fc060df4c1ce8cc5369f0 (2024-06-24)

Dependency updates:
 * Bump golang.org/x/text from 0.15.0 to 0.16.0 ([#1482](#1482)).
 * Bump golang.org/x/term from 0.20.0 to 0.21.0 ([#1483](#1483)).
 * Bump golang.org/x/mod from 0.17.0 to 0.18.0 ([#1484](#1484)).
 * Bump golang.org/x/oauth2 from 0.20.0 to 0.21.0 ([#1485](#1485)).
 * Bump github.com/briandowns/spinner from 1.23.0 to 1.23.1 ([#1495](#1495)).
 * Bump github.com/spf13/cobra from 1.8.0 to 1.8.1 ([#1496](#1496)).
 * Bump github.com/databricks/databricks-sdk-go from 0.42.0 to 0.43.0 ([#1522](#1522)).
@pietern pietern mentioned this pull request Jun 26, 2024
github-merge-queue bot pushed a commit that referenced this pull request Jun 26, 2024
CLI:
* Add link to documentation for Homebrew installation to README
([#1505](#1505)).
* Fix `databricks configure` to use `DATABRICKS_CONFIG_FILE` environment
variable if exists as config file
([#1325](#1325)).

Bundles:

The Terraform upgrade to v1.48.0 includes a fix for library order not
being respected.

* Fix conditional in query in `default-sql` template
([#1479](#1479)).
* Remove user credentials specified in the Git origin URL
([#1494](#1494)).
* Serialize dynamic value for `bundle validate` output
([#1499](#1499)).
* Override variables with lookup value even if values has default value
set ([#1504](#1504)).
* Pause quality monitors when "mode: development" is used
([#1481](#1481)).
* Return `fs.ModeDir` for Git folders in the workspace
([#1521](#1521)).
* Upgrade TF provider to 1.48.0
([#1527](#1527)).
* Added support for complex variables
([#1467](#1467)).

Internal:
* Add randIntn function
([#1475](#1475)).
* Avoid multiple file tree traversals on bundle deploy
([#1493](#1493)).
* Clean up unused code
([#1502](#1502)).
* Use `dyn.InvalidValue` to indicate absence
([#1507](#1507)).
* Add ApplyPythonMutator
([#1430](#1430)).
* Set bool pointer to disable lock
([#1516](#1516)).
* Allow the any type to be set to nil in `convert.FromTyped`
([#1518](#1518)).
* Properly deal with nil values in `convert.FromTyped`
([#1511](#1511)).
* Return `dyn.InvalidValue` instead of `dyn.NilValue` when errors happen
([#1514](#1514)).
* PythonMutator: replace stdin/stdout with files
([#1512](#1512)).
* Add context type and value to path rewriting
([#1525](#1525)).

API Changes:
 * Added schedule CRUD commands to `databricks lakeview`.
 * Added subscription CRUD commands to `databricks lakeview`.
 * Added `databricks apps start` command.

OpenAPI commit 7437dabb9dadee402c1fc060df4c1ce8cc5369f0 (2024-06-24)

Dependency updates:
* Bump golang.org/x/text from 0.15.0 to 0.16.0
([#1482](#1482)).
* Bump golang.org/x/term from 0.20.0 to 0.21.0
([#1483](#1483)).
* Bump golang.org/x/mod from 0.17.0 to 0.18.0
([#1484](#1484)).
* Bump golang.org/x/oauth2 from 0.20.0 to 0.21.0
([#1485](#1485)).
* Bump github.com/briandowns/spinner from 1.23.0 to 1.23.1
([#1495](#1495)).
* Bump github.com/spf13/cobra from 1.8.0 to 1.8.1
([#1496](#1496)).
* Bump github.com/databricks/databricks-sdk-go from 0.42.0 to 0.43.0
([#1522](#1522)).
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.

4 participants