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

Ensure profile flag is respected for sync command #837

Merged
merged 4 commits into from
Oct 9, 2023

Conversation

andrewnester
Copy link
Contributor

@andrewnester andrewnester commented Oct 3, 2023

Changes

Fixes #836

Tests

Manually running sync command with and without the flag

Integration tests pass as well

--- PASS: TestAccSyncFullFileSync (13.38s)
PASS
coverage: 39.1% of statements in ./...
ok      github.com/databricks/cli/internal      14.148s coverage: 39.1% of statements in ./...


--- PASS: TestAccSyncIncrementalFileSync (11.38s)
PASS
coverage: 39.1% of statements in ./...
ok      github.com/databricks/cli/internal      11.674s coverage: 39.1% of statements in ./...

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.

Please also use MustWorkspaceClient in the autocompletion function.

Otherwise, we risk folks autocompleting paths for a different workspace than we use when actually running the command.

Worth checking if autocompletion actually works and doesn't hang, in case the prompting logic appears (this is triggered by MustWorkspaceClient if running interactively).

@andrewnester
Copy link
Contributor Author

@pietern confirmed that autocompletion now works correctly and completes based on the profile defined. Also added a change to not try to prompt for workspace profile

@andrewnester andrewnester requested a review from pietern October 6, 2023 11:12
cmd/root/auth.go Show resolved Hide resolved
cmd/sync/sync.go Outdated Show resolved Hide resolved
@andrewnester andrewnester requested a review from pietern October 9, 2023 10:14
@andrewnester andrewnester added this pull request to the merge queue Oct 9, 2023
Merged via the queue into main with commit ad4b476 Oct 9, 2023
@andrewnester andrewnester deleted the sync-profile-flag branch October 9, 2023 10:43
andrewnester added a commit that referenced this pull request Oct 11, 2023
CLI:
 * Improve `workspace import` command by allowing references to local files for content ([#793](#793)).
 * Add `--file` flag to workspace export command ([#794](#794)).
 * Ensure profile flag is respected for sync command ([#837](#837)).
 * Add hint to delete sync snapshot if parsing fails ([#853](#853)).
 * Use profile information when getting a token using the CLI ([#855](#855)).

Bundles:
 * Minor template tweaks ([#832](#832)).
 * Fixed using repo files as pipeline libraries ([#847](#847)).
 * Support .gitignore syntax in sync section and make sure it works recursively ([#854](#854)).
 * Allow target overrides for sync section ([#856](#856)).

Internal:
 * Fix import export integration tests on windows ([#842](#842)).
 * Fix workspace import test ([#844](#844)).
 * Automatically create a release PR in homebrew-tap repo ([#841](#841)).

Dependency updates:
 * Bump golang.org/x/term from 0.12.0 to 0.13.0 ([#852](#852)).
 * Bump golang.org/x/mod from 0.12.0 to 0.13.0 ([#851](#851)).
 * Bump golang.org/x/sync from 0.3.0 to 0.4.0 ([#849](#849)).
 * Bump golang.org/x/oauth2 from 0.12.0 to 0.13.0 ([#850](#850)).
@andrewnester andrewnester mentioned this pull request Oct 11, 2023
github-merge-queue bot pushed a commit that referenced this pull request Oct 11, 2023
CLI:
* Improve `workspace import` command by allowing references to local
files for content ([#793](#793)).
* Add `--file` flag to workspace export command
([#794](#794)).
* Ensure profile flag is respected for sync command
([#837](#837)).
* Add hint to delete sync snapshot if parsing fails
([#853](#853)).
* Use profile information when getting a token using the CLI
([#855](#855)).

Bundles:
* Minor template tweaks
([#832](#832)).
* Fixed using repo files as pipeline libraries
([#847](#847)).
* Support .gitignore syntax in sync section and make sure it works
recursively ([#854](#854)).
* Allow target overrides for sync section
([#856](#856)).

Internal:
* Fix import export integration tests on windows
([#842](#842)).
* Fix workspace import test
([#844](#844)).
* Automatically create a release PR in homebrew-tap repo
([#841](#841)).


Dependency updates:
* Bump golang.org/x/term from 0.12.0 to 0.13.0
([#852](#852)).
* Bump golang.org/x/mod from 0.12.0 to 0.13.0
([#851](#851)).
* Bump golang.org/x/sync from 0.3.0 to 0.4.0
([#849](#849)).
* Bump golang.org/x/oauth2 from 0.12.0 to 0.13.0
([#850](#850)).
hectorcast-db pushed a commit that referenced this pull request Oct 13, 2023
## Changes
Fixes #836 

## Tests
Manually running `sync` command with and without the flag

Integration tests pass as well

```
--- PASS: TestAccSyncFullFileSync (13.38s)
PASS
coverage: 39.1% of statements in ./...
ok      github.com/databricks/cli/internal      14.148s coverage: 39.1% of statements in ./...


--- PASS: TestAccSyncIncrementalFileSync (11.38s)
PASS
coverage: 39.1% of statements in ./...
ok      github.com/databricks/cli/internal      11.674s coverage: 39.1% of statements in ./...
```
hectorcast-db pushed a commit that referenced this pull request Oct 13, 2023
CLI:
* Improve `workspace import` command by allowing references to local
files for content ([#793](#793)).
* Add `--file` flag to workspace export command
([#794](#794)).
* Ensure profile flag is respected for sync command
([#837](#837)).
* Add hint to delete sync snapshot if parsing fails
([#853](#853)).
* Use profile information when getting a token using the CLI
([#855](#855)).

Bundles:
* Minor template tweaks
([#832](#832)).
* Fixed using repo files as pipeline libraries
([#847](#847)).
* Support .gitignore syntax in sync section and make sure it works
recursively ([#854](#854)).
* Allow target overrides for sync section
([#856](#856)).

Internal:
* Fix import export integration tests on windows
([#842](#842)).
* Fix workspace import test
([#844](#844)).
* Automatically create a release PR in homebrew-tap repo
([#841](#841)).


Dependency updates:
* Bump golang.org/x/term from 0.12.0 to 0.13.0
([#852](#852)).
* Bump golang.org/x/mod from 0.12.0 to 0.13.0
([#851](#851)).
* Bump golang.org/x/sync from 0.3.0 to 0.4.0
([#849](#849)).
* Bump golang.org/x/oauth2 from 0.12.0 to 0.13.0
([#850](#850)).
github-merge-queue bot pushed a commit that referenced this pull request Oct 19, 2023
## Changes

The first stab at this was added in #837 but only included the
`NoPrompt` check in `MustAccountClient`. I renamed it to `SkipPrompt`
(in preparation for another option that skips bundle load) and made it
work for `MustWorkspaceClient` as well.

## Tests

Manually confirmed that the completion hook no longer prompts for a
profile (when called directly with `databricks __complete`).
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.

profile flag seems to be ignored in databricks sync command
2 participants