Skip to content

Commit fa16d01

Browse files
denikIsaac
andauthored
Update docs and comments for the direct-only engine (#6939)
## Changes Docs, comments, schema descriptions and agent guides still described two deployment engines. - `docs/direct.md`: Terraform state is migrated automatically; `engine: terraform` errors (stay on CLI v1.19.x to keep Terraform); fix backup file names. - Drop "direct engine only" qualifiers from `libraries`/`lifecycle.started` schema descriptions (schema + Python docstrings regenerated) and from `--plan` help in `bundle deploy`/`apps deploy`. - Stale comments referring to tfdyn, terraform deploys or both engines; explain why the empty apps `description` is still kept (removing a description from config must clear it remotely). - Agent guides/skills: remove the two-engine matrix instructions. This pull request and its description were written by Isaac. --------- Co-authored-by: Isaac <no-reply@databricks.com>
1 parent 755fb9f commit fa16d01

28 files changed

Lines changed: 67 additions & 86 deletions

File tree

‎.agents/rules/telemetry.md‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ The deploy payload is declared twice: the proto in the universe repo (`proto/log
2525
## Adding a new metric
2626

2727
1. Add the field to the relevant message in `libs/telemetry/protos/`, and populate it in `bundle/phases/telemetry.go`.
28-
2. Refresh the acceptance golden: `go test ./acceptance -run '^TestAccept/bundle/telemetry' -update`. Anything engine-dependent belongs in a per-engine `out.*.$DATABRICKS_BUNDLE_ENGINE.txt` file rather than the engine-agnostic `out.telemetry.txt`; see the comments in that test's `script`.
28+
2. Refresh the acceptance golden: `go test ./acceptance -run '^TestAccept/bundle/telemetry' -update`.
2929
3. Send the universe PR adding the same field to the proto, with a `compliance.data_label` annotation matching the neighbouring fields.
3030

3131
**RULE: Never send a resource name, path, or any other user-authored string.** Names are PII. Resource identity goes through the numeric/UUID ID lists (`resource_job_ids` and friends), and only for types that have such an ID.
@@ -37,7 +37,7 @@ Other things worth knowing:
3737
- A one-off boolean needs no proto change: add a key constant to `bundle/metrics/metrics.go` (snake_case, matching the constant name) and call `b.Metrics.SetBoolValue(...)`. It lands in `experimental.bool_values`.
3838
- Metrics that may be removed later belong in `BundleDeployExperimental`, which carries no compatibility guarantees.
3939
- Positional encodings such as the `upload_file_sizes` histogram are frozen once the metric has adoption: changing a bucket bound silently changes the meaning of every previously reported entry.
40-
- Anything measured from the deployment state is engine-dependent. The direct engine keeps per-resource state in `resources.json` and reports sizes; the terraform path leaves `StateSizeBytes` zero and reports only counts.
40+
- Per-resource state sizes come from the direct engine's state in `resources.json`.
4141

4242
## Where the data lands
4343

‎.agents/skills/bump-sdk/SKILL.md‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -50,8 +50,8 @@ go test ./acceptance -run '^TestAccept$' -timeout=60m # MUST pass on its o
5050
```
5151

5252
The verify pass is not optional.
53-
Bundle tests run under an `EnvMatrix` of both engines (`terraform`, `direct`).
54-
When a schema change makes one engine error while the other succeeds, the variants produce different output; `-update` runs both and each overwrites the other's `output.txt`, so it can silently settle on the passing variant and report `ok` while the golden is actually wrong.
53+
Many bundle tests run under an `EnvMatrix` (e.g. `DMS`, `READPLAN`).
54+
When a schema change makes one variant error while another succeeds, the variants produce different output; `-update` runs all of them and each overwrites the other's `output.txt`, so it can silently settle on the passing variant and report `ok` while the golden is actually wrong.
5555
Only the non-update run catches this.
5656
(Ignore `rejecting_proxy.go: blocking proxy` log lines, which are normal.
5757
A test that times out under full parallel load but passes when run alone is a flake, not a regression.)

‎.agents/skills/pr-checklist/SKILL.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ Before submitting a PR, run these commands to match what CI checks. CI uses the
1313
# 2. Linting (CI runs full golangci-lint across all modules, not the diff-only wrapper)
1414
./task lint
1515

16-
# 3. Tests (CI runs with both deployment engines)
16+
# 3. Tests
1717
./task test
1818

1919
# 4. If you changed bundle config structs, schema, or direct-engine resource code:

‎AGENTS.md‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,7 @@ GIT_EDITOR=true GIT_SEQUENCE_EDITOR=true VISUAL=true GIT_PAGER=cat git rebase or
6868
**bundle/** - Core bundle functionality for Declarative Automation Bundles
6969
- `bundle/bundle.go` - Main Bundle struct and lifecycle management
7070
- `bundle/config/` - Configuration loading, validation, and schema
71-
- `bundle/deploy/` - Deployment logic (Terraform and direct modes)
71+
- `bundle/deploy/` - Deployment logic (file upload, locking, state)
7272
- `bundle/mutator/` - Configuration transformation pipeline
7373
- `bundle/phases/` - High-level deployment phases
7474

@@ -84,9 +84,7 @@ GIT_EDITOR=true GIT_SEQUENCE_EDITOR=true VISUAL=true GIT_PAGER=cat git rebase or
8484

8585
**Mutators**: Transform bundle configuration through a pipeline. Located in `bundle/config/mutator/` and `bundle/mutator/`. Each mutator implements the `Mutator` interface.
8686

87-
**Direct vs Terraform Deployment**: The CLI supports two deployment modes controlled by `DATABRICKS_BUNDLE_ENGINE` environment variable:
88-
- `direct` (default) - Direct API calls without Terraform
89-
- `terraform` - Uses Terraform for resource management
87+
**Direct deployment engine**: Bundles are deployed by the direct engine (`bundle/direct/`), which calls the APIs without Terraform. The Terraform engine was removed in v1.20.0: `bundle.engine: terraform` and `DATABRICKS_BUNDLE_ENGINE=terraform` are rejected, and existing Terraform state is migrated to the direct engine before the command runs.
9088

9189
# Development Tips
9290

‎acceptance/bundle/help/bundle-deploy/output.txt‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ Flags:
1919
--force Force-override Git branch validation.
2020
--force-lock Force acquisition of deployment lock.
2121
-h, --help help for deploy
22-
--plan string Path to a JSON plan file to apply instead of planning (direct engine only).
22+
--plan string Path to a JSON plan file to apply instead of planning.
2323
-q, --quiet count Reduce output: -q prints only the summary, -qq prints only warnings and errors.
2424
--select strings Deploy only the specified resource (e.g. 'my_job' or 'jobs.my_job'). Can be repeated or comma-separated.
2525

‎bundle/bundle.go‎

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -86,7 +86,7 @@ type Metrics struct {
8686
// ResourceState is the direct engine's per-resource deployment state
8787
// captured right after the deploy. It carries each resource's state-size in
8888
// bytes so deploy telemetry can be derived without re-reading or re-parsing
89-
// the state file. Nil for terraform deploys.
89+
// the state file.
9090
ResourceState resourcestate.ExportedResourcesMap
9191
}
9292

@@ -188,12 +188,10 @@ type Bundle struct {
188188
// When non-empty, only the specified resources are included in deployment.
189189
Select []string
190190

191-
// MigratingToDirect is set when the direct engine is requested but the existing
192-
// state still uses terraform, so the state is migrated to the direct engine after
193-
// this deploy. Resources that only the direct engine supports are skipped by this
194-
// run rather than rejected: terraform cannot deploy them, and since terraform
195-
// could never have deployed them they are absent from its state. The next deploy,
196-
// which runs on the migrated state, creates them.
191+
// MigratingToDirect is set when the existing state still uses terraform, so it is
192+
// migrated to the direct engine in memory before the command runs (statemgmt.Migrate).
193+
// Deploy and destroy commit the migration (statemgmt.CommitMigration); other commands
194+
// leave the terraform state untouched.
197195
MigratingToDirect bool
198196

199197
// Quiet is the output verbosity reduction requested via -q/--quiet, which is

‎bundle/config/bundle.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -46,8 +46,8 @@ type Bundle struct {
4646
// Overrides the cluster used for jobs and other supported assets.
4747
ClusterId string `json:"cluster_id,omitempty"`
4848

49-
// Engine specifies the deployment engine to use ("terraform" or "direct").
50-
// Defaults to "direct"; set "terraform" to opt out.
49+
// Engine specifies the deployment engine to use. "direct" is the only supported
50+
// engine and the default; "terraform" is rejected with the removal error.
5151
// Can be overridden with the DATABRICKS_BUNDLE_ENGINE environment variable.
5252
Engine engine.EngineType `json:"engine,omitempty"`
5353

‎bundle/config/engine/engine.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ const (
1818
)
1919

2020
// Default is used when the user has not set the value, both for new bundles and
21-
// for existing terraform deployments (which are migrated to it after a deploy).
21+
// for existing terraform deployments (whose state is migrated to it before the command runs).
2222
const Default = EngineDirect
2323

2424
// TerraformRemovedMessage is the error text shown when a bundle pins the removed

‎bundle/config/experimental.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ type Experimental struct {
77
// single immutable snapshot rather than being synced individually. When true,
88
// the deployment calls /api/2.0/repos/snapshots with a zip of all files and sets
99
// workspace.file_path and workspace.artifact_path to the returned content-addressed
10-
// path. Only supported with the direct deployment engine.
10+
// path.
1111
ImmutableFolder bool `json:"immutable_folder,omitempty"`
1212

1313
// By default Python wheel tasks deployed as is to Databricks platform.

‎bundle/config/mutator/drop_empty_strings.go‎

Lines changed: 6 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -17,10 +17,10 @@ type dropEmptyStrings struct{}
1717
// either literally (policy_id: "") or via a variable that resolved to "", so
1818
// this must run after variable resolution.
1919
//
20-
// Both engines convert the resolved config through convert.ToTyped, which
20+
// The direct engine converts the resolved config through convert.ToTyped, which
2121
// force-sends explicitly-set zero values, defeating the omitempty tag. Dropping
22-
// here fixes it uniformly for terraform and direct and makes the result visible
23-
// in `bundle validate -o json`, which serializes the dynamic value.
22+
// here fixes it and makes the result visible in `bundle validate -o json`, which
23+
// serializes the dynamic value.
2424
func DropEmptyStrings() bundle.Mutator {
2525
return &dropEmptyStrings{}
2626
}
@@ -43,11 +43,9 @@ func (m *dropEmptyStrings) Apply(ctx context.Context, b *bundle.Bundle) diag.Dia
4343
return root, err
4444
}
4545

46-
// It seems safe to send an empty description "" to the backend, but for apps
47-
// it causes drift in terraform: the Apps API always returns "" for an unset
48-
// description, so bundle/deploy/terraform/tfdyn/convert_app.go injects it
49-
// anyway. To avoid an unnecessary difference between the direct and terraform
50-
// engines, keep the empty apps description here instead of dropping it.
46+
// Keep an empty apps description instead of dropping it: an app update without
47+
// the field leaves the remote description unchanged, so removing description
48+
// from config would never converge. Force-sending "" clears it.
5149
return dyn.MapByPattern(root, dyn.NewPattern(dyn.Key("resources"), dyn.Key("apps"), dyn.AnyKey()), func(_ dyn.Path, app dyn.Value) (dyn.Value, error) {
5250
if _, err := dyn.Get(app, "description"); err != nil {
5351
return dyn.Set(app, "description", dyn.V(""))

0 commit comments

Comments
 (0)