bundle deploy/destroy: report per-resource actions and a summary - #5720
Conversation
Integration test reportCommit: 3fce466
10 interesting tests: 4 SKIP, 3 flaky, 3 RECOVERED
Top 30 slowest tests (at least 2 minutes):
|
2e8f647 to
5d6d622
Compare
5d6d622 to
139c45e
Compare
c56ed07 to
f8baedd
Compare
d88ce7f to
fd2b017
Compare
4bee1f1 to
74bd52e
Compare
74bd52e to
d1e61ac
Compare
| @@ -40,6 +41,7 @@ See https://docs.databricks.com/en/dev-tools/bundles/index.html for more informa | |||
| cmd.Flags().BoolVar(&autoApprove, "auto-approve", false, "Skip interactive approvals that might be required for deployment.") | |||
| cmd.Flags().MarkDeprecated("compute-id", "use --cluster-id instead") | |||
| cmd.Flags().BoolVar(&verbose, "verbose", false, "Enable verbose output.") | |||
| cmd.Flags().BoolVarP(&quiet, "quiet", "q", false, "Only print the summary line, not the per-resource actions.") | |||
There was a problem hiding this comment.
The PR description says repeating the flag (-qq) prints "only warnings and errors, which also drops progress messages (Uploading bundle files to ..., Building ..., Executing 'postdeploy' script)". But --quiet is a plain bool here (and in plan.go / pipelines/deploy.go), and b.Quiet in bundle.go is a bool whose only consumer is the if !b.Quiet guard in logDeploySummary, which gates just the per-resource lines.
pflag parses -qq as setting the bool twice, so -qq is identical to -q. Running bundle deploy -qq against a live workspace printed the Uploading bundle files to ... progress line and the full Files:/Resources: summary, byte-for-byte the same as -q (a changed resource still showed everything except the per-resource line). The second quiet level described in the PR body isn't implemented — either trim the description, or make this a repeat counter (e.g. CountVarP) and wire progress suppression through cmdio.
This review comment was added by Isaac.
There was a problem hiding this comment.
Right, BoolVarP made -qq identical to -q. Now a CountVarP on deploy/plan/destroy/pipelines deploy (destroy had no -q before), with progress suppression through cmdio.WithQuiet/LogProgress as you suggested; LogString is untouched since it also carries results.
acceptance/bundle/quiet-levels asserts all nine command/level combinations, and shared-root-path / create-error show -qq still printing warnings and errors.
One exception: -qq hides destroy's deletion listing only with --auto-approve; without it we are about to ask for consent, so it always prints (TestApprovalForDestroyQuietWhilePrompting — needs a TTY, so not an acceptance test).
This reply was added by Claude Code.
| if plan.NotSelected > 0 { | ||
| summary += fmt.Sprintf(", %d not selected", plan.NotSelected) | ||
| } | ||
| cmdio.LogString(ctx, summary+".") |
There was a problem hiding this comment.
This Resources: ... unchanged. summary line ends with a period (summary+"."), which is inconsistent with the sibling Files: %d uploaded, %d deleted line just above it (line 151, no period) and with the Plan: %d to add, ... %d unchanged line in cmd/bundle/plan.go (also no period). Within this PR, Resources: and Destroy: %d deleted. end with a period while Files: and Plan: don't — worth making the status lines uniform one way or the other.
This review comment was added by Isaac.
There was a problem hiding this comment.
Agreed — dropped from Files:, Resources: and Destroy: so all four status lines match Plan:. The same change removes the conditions that hid these lines, so the output shape no longer varies between deploys, which also covers the blank-line point from your first review.
This reply was added by Claude Code.
3336d7b to
50eb18d
Compare
`bundle deploy` now lists the per-resource actions and a summary line,
mirroring `bundle plan`, instead of generic progress chatter:
created jobs.foo
deleted pipelines.bar
Deploy: 1 created, 0 changed, 1 deleted, 3 unchanged.
`bundle destroy` gets a matching summary line (`Destroy: N deleted.`),
counting top-level resources to match its approval list.
Details:
- Add CountActions()/ActionCounts and NotSelected to deployplan.Plan.
- Add --quiet/-q to plan, deploy and pipelines deploy to print only the
summary line.
- With --select, the summary appends ", N not selected".
- Drop progress chatter: "Deploying resources...", "Deployment complete!",
"Updating deployment state...", "Deleting files...", "Destroy complete!".
A few acceptance tests where the engines genuinely diverge per-resource
(secret-scope/grant/permission sub-resources, cluster resize verb, no-op
update detection) use `deploy -q | grep -v '^Deploy:'` to stay engine-stable.
Co-authored-by: Isaac
The rebase landed the NotSelected assignment inside enqueueReachable instead of at the end of FilterToSelected. Move it back so it runs once after the BFS prune completes. Co-authored-by: Isaac
The rebase pulled in many acceptance tests added on main after this branch point (postgres_*, genie_spaces, migrate variants, etc.) that exercise deploy/destroy output. Regenerate their goldens for the new per-resource-action + summary format. Also normalize the Destroy: count in the delete-trashed-out-of-band dashboard test via a Repls entry: terraform refreshes the trashed dashboard away and reports "0 deleted" while direct still counts it, so the count diverges per engine. Co-authored-by: Isaac
mlops-stacks is a Cloud-only test (Local=false), so ./task test-update cannot regenerate it locally. Hand-update its golden to the new per-resource-action + summary format: 4 fresh creates and a matching Destroy: 4 deleted line. Resource order matches the destroy list already in the golden. This was the only integration-test failure after the deploy/destroy output change. Co-authored-by: Isaac
Regenerated against AWS cloud via deco. The prior hand-edit undercounted: each resource also deploys a .permissions or .grants sub-node, so deploy reports 8 created, not 4. Destroy still counts the 4 top-level resources. Co-authored-by: Isaac
…table Tests added on main still had the old progress output; regenerate them to the per-resource + summary format. In grants/schemas/remove_all the engines classify emptying the grants list differently (terraform reports the grants node as deleted, direct as updated), so that deploy uses "-q | grep -v '^Deploy:'" to stay engine-stable, matching all_privileges_coexist. Co-authored-by: Isaac
Co-authored-by: Isaac
Co-authored-by: Isaac
…:, capitalize verbs
Reviewer feedback on the deploy summary:
- The blank line before the summary only appeared when there were per-resource
lines, so the output shape varied between deploys. Dropped.
- "0 changed" looked like a no-op when only business logic (a .py or .sql file)
changed. Report file sync separately as "Files: N uploaded, N deleted", printed
only when something was synced. libs/sync now keeps the counts from its diff;
b.Files lists everything tracked, not what changed.
- Per-resource verbs are capitalized ("Created jobs.foo") to match the sentence
case of the surrounding output. "bundle plan" keeps the lower-case present
tense, so the two commands stay distinguishable.
The summary line is now "Resources:" rather than "Deploy:", since it counts
resources and sits next to a "Files:" line. It is omitted entirely when the plan
is empty. Tests that filtered '^Deploy:' to hide engine-divergent counts now
filter '^Resources:' and let the engine-independent Files: line through.
The new "Files: N uploaded, N deleted" line asserts how many files the deploy synced, which surfaced two fixtures whose synced file set was not reproducible: - bundle/python: requirements-latest.txt was created only for PYDAB_VERSION=current, so that variant synced one more file than the pinned one while both share a single output.txt. Create it in both branches. - bundle/apps/job_permissions: the script writes out.after_first_deploy.* between deploys, so a later deploy's count depended on which engine variant ran before. Exclude out.* from sync.
The test is gated to Windows (it runs cmd.exe), so -update on macOS/Linux cannot refresh it. The new value is taken from its bash/sh/default siblings, which have an identical file set and now all record "Files: 6 uploaded, 0 deleted".
--quiet becomes repeatable: -q prints only the summary lines, -qq additionally
drops them along with progress messages ("Uploading bundle files to ...",
"Building ...", "Executing 'postdeploy' script"). Warnings and errors are never
suppressed: they go through libs/logdiag, not cmdio.
Progress messages are emitted by mutators that receive only a context, not the
bundle, so the level travels on the context via cmdio.WithQuiet/LogProgress.
LogString is left alone, since it also carries results the user asked for.
bundle destroy gains -q; plan/deploy/pipelines deploy switch their bool flag to
a count. Also drops the trailing period from the summary lines so Files:,
Resources: and Destroy: punctuate alike.
The grep pipelines existed to hide summary lines that differ between engines. -qq does it directly, so the intent is visible at the call site. The two clusters/deploy tests keep -q: only their per-resource verb diverges (direct reports resize, terraform update), and the Resources: summary agrees, so it is still worth asserting. Documented in each script.
Drop the conditions that hid "Files:" when nothing synced and "Resources:" when the plan was empty, so the output has the same shape on every deploy. delete-trashed-out-of-band drops its Repls rule for the divergent destroy count and passes -qq instead, with the reason stated in the script.
destroy -qq still printed the deletion listing and the root-path line, so it did not match deploy -qq. It is now silent -- but only with --auto-approve: without it we are about to ask for consent and the user must see what they are consenting to, so the listing prints at any -q level. logPipelineDeleteApproval still runs when quiet, since its cascade lookup can fail and that error has to surface; only its printing is suppressed. acceptance/bundle/deploy/quiet-levels now asserts all nine combinations of plan/deploy/destroy x default/-q/-qq. The prompting case needs a TTY, which acceptance tests do not have, so it is covered by a unit test.
Each level now starts from nothing deployed, with a destroy in between, so the three outputs differ only by the flag. Before, the second deploy had nothing left to create and the third nothing to change, so the counts differed for that reason instead of because of -q. Deploy an experiment rather than a notebook job: no local files to sync, no language runtime leaving __pycache__ behind, and one plan node on both engines (secret scopes model permissions as a plan node on direct but not terraform, which made the counts diverge).
The direct-only migration tests from #6261 landed on main while this branch was in review, so their goldens still carried the old "Deploying resources..." output.
The files are uploaded before planning and applying, so a deploy that fails later still synced them and "Files:" is accurate. It is now printed on those paths too, via a defer armed right after the upload so a new early return cannot drop it. "Resources:" stays absent there: a failure before planning has no counts to report, and one after it would describe what was intended rather than what was applied. Renamed the test that asserts this to partial-summary-on-push-fail. jobs/shared-root-path and jobs/create-error now deploy at each quiet level, showing that -q and -qq still print warnings and errors.
50eb18d to
3fce466
Compare
The deploy summary from #5720 counts uploaded files, and repls.json was one of them. Co-authored-by: Isaac
Integration test reportCommit: 6444cb8
17 interesting tests: 9 FAIL, 4 KNOWN, 2 RECOVERED, 2 SKIP
Top 50 slowest tests (at least 2 minutes):
|
Match the per-resource deploy/destroy output from #5720 after merging main.
Match the per-resource deploy/destroy output from #5720 after merging main.
…ctory (databricks#6264) ## Why Both files were created inside the test directory, so bundle deploy uploaded them to the workspace. They also carried the same data in two formats: repls.json from the harness to diff.py, ACC_REPLS from the scripts back to the harness. Having it in test directory made file count less stable in databricks#5720 ## Changes A single file at $ACC_REPLS, outside the test directory. Every line is one replacement encoded as a JSON object: "Old" for a regular expression from the harness, "Literal" for a value added by add_repl.py. Scripts no longer write the file directly - they use the new add_repl helper, which works under MSYS_NO_PATHCONV=1. diff.py and sort_lines.py --repl now apply the script replacements too.
## Why `dashboards/simple_syncroot` and `dashboards/simple_outside_bundle_root` set `sync.paths` to one level above the bundle root, so they sync the acceptance tmpdir's parent. Its contents differ per environment, so `Files: N uploaded` is not stable: the golden says 9, DBR reports 6. Both tests pass in the six plain integration envs and fail in the two `dbr` jobs, since #5720 added the line. ## Changes Add `-qq` to the deploy. The count is noise here — these tests assert dashboard path resolution via `lakeview get`.
## Release v1.13.0 ### Notable Changes * `bundle deploy` now reports the per-resource actions it took, how many files it synced, and a summary of created/changed/deleted/unchanged resources; `bundle destroy` reports how many resources it deleted. `-q` prints only the summaries, `-qq` only warnings and errors. ([#5720](#5720)) ### CLI * `databricks aitools install` now supports Goose, installing Databricks agent skills into its skills directory. * Error messages for failed key lookups and variable references now suggest the closest matching key if one is found. ([#6208](#6208)) * Released binaries are now built against the FIPS 140-3 validated Go Cryptographic Module, with FIPS 140-3 mode enabled by default. TLS connections negotiate only FIPS-approved cipher suites, which drops ChaCha20 and CBC from what the client offers. FIPS mode can be disabled at startup with `GODEBUG=fips140=off`, which restores the previous TLS behaviour ([#6262](#6262)). * `databricks environments setup-local` now removes a `databricks-connect` pin from `[project].dependencies`, an optional-dependency extra, or a dependency group when its version range conflicts with the compute target's `databricks-connect` version, so `uv sync` no longer fails with an unsatisfiable resolution when a template ships a conflicting pin. A pin that co-resolves, carries no version, or is marker-gated is left untouched, and each removed pin is reported with the new `W_DBCONNECT_CONSOLIDATED` warning. Wildcard version pins such as `==15.1.*` are now also checked for conflicts with the environment's constraints. ### Bundles * Allow dashes in the catalog and schema names prompted by `databricks bundle init`, and backtick-quote the catalog and schema identifiers in the SQL generated by the built-in templates so names with dashes work at runtime. * Fixed `bundle.git.branch`, `bundle.git.commit`, and `bundle.git.origin_url` being empty for bundles deployed from a workspace Git folder that has Git CLI access. The workspace API does not report git metadata for those folders, so it is now read from the Repos API instead. * direct: job_runs deploy progress lines now include the resource key (e.g. `Output from job_runs.foo: id=123: ...`) so concurrent runs are easier to tell apart. * direct: `resources.job_runs` can set `lifecycle.triggers.on_bundle_deploy: true` to re-fire the run on every bundle deploy. Removing the trigger does not recreate the existing run. * When migrating a bundle to the direct deployment engine, resources that only the direct engine supports (e.g. instance pools, catalogs) are now skipped by the deploy that migrates the state instead of failing it. They are created by the next deploy, which runs on the migrated state. * Warn on invalid `secret_scopes` permission levels (`READ`, `WRITE`, `MANAGE`); fail under `bundle validate --strict`. * Reject secret scope permissions that name no principal, instead of failing after the scope is created. * Write the deployment state atomically so an interrupted save cannot leave a state file that the CLI refuses to read. * Warn when the deployment state was last written by a newer CLI version than the one running. * Support pip extras (e.g. `[train]`) on local wheels in a job environment's `dependencies` ([#1602](#1602)). ### Dependency Updates * Bump `github.com/databricks/databricks-sdk-go` from v0.170.0 to v0.171.0 ([#6320](#6320)). * Bump the Databricks Terraform provider to 1.127.0 ([#6319](#6319)). * Bump Go toolchain to 1.26.6 ([#6266](#6266)). * Bump Go toolchain to 1.26.7 ([#6325](#6325)).
bundle deploynow lists the per-resource actions and summary lines, instead of generic progress chatter:Files:is reported separately because a deploy that only changes business logic (a.pyor.sqlfile) leaves every resource unchanged, so a lone0 changedreads like a no-op. Both lines always print, so the shape of the output does not vary between deploys.A deploy that fails after uploading still reports
Files:— those files were uploaded. It does not reportResources:, which would describe what was intended rather than what was applied.bundle destroygets a matching summary line (Destroy: N deleted), counting top-level resources to match its approval list.Details
CountActions()/ActionCountsandNotSelectedtodeployplan.Plan.libs/syncretains the upload/delete counts from its diff;b.Fileslists every tracked file, not the changed ones.-q/--quietprints only the summary lines. Repeat it (-qq) to print only warnings and errors, which also drops progress messages (Uploading bundle files to ...,Building ...,Executing 'postdeploy' script). Supported onplan,deploy,destroyandpipelines deploy;destroydid not have-qbefore. It is a repeat counter, and progress suppression travels on the context (cmdio.WithQuiet/LogProgress) because those messages come from mutators that receive no bundle.acceptance/bundle/quiet-levelscovers all nine command/level combinations; acceptance tests use-qqwhere the engines genuinely diverge per-resource.--select, the resource summary appends, N not selected.Deploying resources...,Deployment complete!,Updating deployment state...,Deleting files...,Destroy complete!.postdeployscript, so a failing script is not preceded by a line that appears to vouch for the run; it still prints on that failure (the resources were applied) and the script's error still propagates.