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

Add "output" flag to the bundle sync command #1853

Merged
merged 3 commits into from
Oct 23, 2024
Merged

Conversation

ilia-db
Copy link
Contributor

@ilia-db ilia-db commented Oct 22, 2024

Changes

We want to use 'bundle sync' in the vscode extension before running a file as an ad-hoc job (or through the context api). Right now we use bundle deploy in these cases, but deploying bundle resources is not always expected when you just want to quickly run a file. Sync makes more sense in these cases, but we still want to have verbose output to see what's happening.

In the 'deploy' command we have hidden 'verbose' flag. For the sync I've just added 'output' flag, handling both json and text cases, similar to how it's done in the non-bundle sync command. The flag is not hidden (although we still don't show any output by default, if the flag is not set).

VSCode Extension PR: databricks/databricks-vscode#1401

Tests

Manually

We want to use 'bundle sync' in the vscode extension before running a file
as an ad-hoc job (or through the context api). Right now we use bundle deploy
in these cases, but deploying bundle resources is not always expected when you
just want to quickly run a file. Sync make more sense in these cases, but we
still want to have verbose output to see what's happening.
@ilia-db ilia-db requested a review from pietern October 22, 2024 11:02
@ilia-db ilia-db marked this pull request as ready for review October 22, 2024 11:02
ilia-db added a commit to databricks/databricks-vscode that referenced this pull request Oct 22, 2024
Also changes Sync logic to use the bundle sync command.

Depends on the CLI PR: databricks/cli#1853
@pietern pietern added this pull request to the merge queue Oct 23, 2024
outputFunc = sync.TextOutput
case flags.OutputJSON:
outputFunc = sync.JsonOutput
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you add a default handler so we error out on unknown types? Smth like

default:
  return fmt.Errorf("unknown output type %s", f.output)

case flags.OutputJSON:
outputFunc = sync.JsonOutput
}
if outputFunc != nil {
Copy link
Contributor

Choose a reason for hiding this comment

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

You can remove this if you have a default handler in the switch above

@@ -65,6 +85,7 @@ func newSyncCommand() *cobra.Command {
if err != nil {
return err
}
defer s.Close()
Copy link
Contributor

Choose a reason for hiding this comment

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

Was this causing any issue before?

Merged via the queue into main with commit 55a055d Oct 23, 2024
5 checks passed
@pietern pietern deleted the ilia-db/bundle-sync-output branch October 23, 2024 11:13
andrewnester added a commit that referenced this pull request Oct 23, 2024
CLI:
 * Added JSON input validation for CLI commands ([#1771](#1771)).

Bundles:
 * Support Git worktrees for `sync` ([#1831](#1831)).
 * Add `bundle summary` to display URLs for deployed resources ([#1731](#1731)).
 * Added a warning when incorrect permissions used for `/Workspace/Shared` bundle root ([#1821](#1821)).
 * Show actionable errors for collaborative deployment scenarios ([#1386](#1386)).
 * Fix path to repository-wide exclude file ([#1837](#1837)).
 * Fixed typo in converting cluster permissions ([#1826](#1826)).
 * Ignore metastore permission error during template generation ([#1819](#1819)).
 * Handle normalization of `dyn.KindTime` into an any type ([#1836](#1836)).
 * Added support for pip options in environment dependencies ([#1842](#1842)).
 * Fix race condition when restarting continuous jobs ([#1849](#1849)).
 * Fix pipeline in default-python template not working for certain workspaces ([#1854](#1854)).
 * Add "output" flag to the bundle sync command ([#1853](#1853)).

Internal:
 * Move utility functions dealing with IAM to libs/iamutil ([#1820](#1820)).
 * Remove unused `IS_OWNER` constant ([#1823](#1823)).
 * Assert SDK version is consistent in the CLI generation process ([#1814](#1814)).
 * Fixed unmarshalling json input into `interface{}` type ([#1832](#1832)).
 * Fix `TestAccFsMkdirWhenFileExistsAtPath` in isolated Azure environments ([#1833](#1833)).
 * Add behavioral tests for examples from the YAML spec ([#1835](#1835)).
 * Remove Terraform conversion function that's no longer used ([#1840](#1840)).
 * Encode assumptions about the dashboards API in a test ([#1839](#1839)).
 * Add script to make testing of code on branches easier ([#1844](#1844)).

API Changes:
 * Added `databricks disable-legacy-dbfs` command group.

OpenAPI commit cf9c61453990df0f9453670f2fe68e1b128647a2 (2024-10-14)
Dependency updates:
 * Upgrade TF provider to 1.54.0 ([#1852](#1852)).
 * Bump github.com/databricks/databricks-sdk-go from 0.48.0 to 0.49.0 ([#1843](#1843)).
github-merge-queue bot pushed a commit that referenced this pull request Oct 23, 2024
CLI:
* Added JSON input validation for CLI commands
([#1771](#1771)).
* Support Git worktrees for `sync`
([#1831](#1831)).

Bundles:
* Add `bundle summary` to display URLs for deployed resources
([#1731](#1731)).
* Added a warning when incorrect permissions used for
`/Workspace/Shared` bundle root
([#1821](#1821)).
* Show actionable errors for collaborative deployment scenarios
([#1386](#1386)).
* Fix path to repository-wide exclude file
([#1837](#1837)).
* Fixed typo in converting cluster permissions
([#1826](#1826)).
* Ignore metastore permission error during template generation
([#1819](#1819)).
* Handle normalization of `dyn.KindTime` into an any type
([#1836](#1836)).
* Added support for pip options in environment dependencies
([#1842](#1842)).
* Fix race condition when restarting continuous jobs
([#1849](#1849)).
* Fix pipeline in default-python template not working for certain
workspaces ([#1854](#1854)).
* Add "output" flag to the bundle sync command
([#1853](#1853)).

Internal:
* Move utility functions dealing with IAM to libs/iamutil
([#1820](#1820)).
* Remove unused `IS_OWNER` constant
([#1823](#1823)).
* Assert SDK version is consistent in the CLI generation process
([#1814](#1814)).
* Fixed unmarshalling json input into `interface{}` type
([#1832](#1832)).
* Fix `TestAccFsMkdirWhenFileExistsAtPath` in isolated Azure
environments ([#1833](#1833)).
* Add behavioral tests for examples from the YAML spec
([#1835](#1835)).
* Remove Terraform conversion function that's no longer used
([#1840](#1840)).
* Encode assumptions about the dashboards API in a test
([#1839](#1839)).
* Add script to make testing of code on branches easier
([#1844](#1844)).

API Changes:
 * Added `databricks disable-legacy-dbfs` command group.

OpenAPI commit cf9c61453990df0f9453670f2fe68e1b128647a2 (2024-10-14)
Dependency updates:
* Upgrade TF provider to 1.54.0
([#1852](#1852)).
* Bump github.com/databricks/databricks-sdk-go from 0.48.0 to 0.49.0
([#1843](#1843)).
ilia-db added a commit to databricks/databricks-vscode that referenced this pull request Oct 24, 2024
…1401)

## Changes
Also changes the continuous sync logic to use the `bundle sync` command
instead of the `sync`.

Depends on the CLI PR: databricks/cli#1853

Sync E2E tests will be failing until we update the CLI

## Tests
Manual and existing unit and e2e tests
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.

3 participants