internal: Refactor immutable folder upload as an internal resource in direct engine by andrewnester · Pull Request #6084 · databricks/cli

andrewnester · 2026-07-28T11:29:51Z

Changes

Refactor immutable folder upload as an internal resource in direct engine

Why

This enables us:

Keep track of upload state and upload paths, so next deployment can check the status of the uploaded folder. Needed for breakglass
Easier path resolution: now all resources can just reference to internal resource like ${internal.snapshot.full_path} and it's automatically resolved during deploy
Plan is correct now and can easily skip uploads if nothing changed and no need to upload

Tests

Existing tests pass

… direct engine

github-actions · 2026-07-28T11:31:44Z

Approval status: pending

`/acceptance/bundle/` - needs approval

18 files changed
Suggested: @denik
Also eligible: @pietern, @janniklasrose, @shreyas-goenka, @lennartkats-db, @anton-107

`/bundle/` - needs approval

33 files changed
Suggested: @denik
Also eligible: @pietern, @janniklasrose, @shreyas-goenka, @lennartkats-db, @anton-107

`/cmd/bundle/` - needs approval

Files: cmd/bundle/plan.go
Suggested: @denik
Also eligible: @pietern, @janniklasrose, @shreyas-goenka, @lennartkats-db, @anton-107

`/libs/sync/` - needs approval

Files: libs/sync/path.go
Suggested: @simonfaltum
Also eligible: @Divyansh-db, @renaudhartert-db, @tanmay-db, @hectorcast-db, @parthban-db, @tejaskochar-db, @mihaimitrea-db, @chrisst, @rauchy

General files (require maintainer)

4 files changed
Based on git history:

@denik -- recent work in bundle/phases/, bundle/direct/dresources/, bundle/direct/

_{Any maintainer (@anton-107, @denik, @pietern, @shreyas-goenka, @simonfaltum, @renaudhartert-db, @janniklasrose, @lennartkats-db, @rugpanov, @rclarey) can approve all areas.

See OWNERS for ownership rules.}

eng-dev-ecosystem-bot · 2026-07-28T11:51:46Z

Integration test report

Commit: b60469f

Run: 32761216334

	Env	🔄flaky	💚RECOVERED	🙈SKIP	✅pass	🙈skip	Time
💚	aws linux		1	4	276	1167	7:46
💚	aws windows		1	4	278	1165	6:36
💚	azure linux		1	4	275	1167	9:58
🔄	azure windows	1	1	4	276	1165	6:50
💚	gcp linux		1	4	276	1167	9:37
💚	gcp windows		1	4	278	1165	7:52

6 interesting tests: 4 SKIP, 1 RECOVERED, 1 flaky

	Test Name	aws linux	aws windows	azure linux	azure windows	gcp linux	gcp windows
💚	TestAccept	💚R	💚R	💚R	💚R	💚R	💚R
🙈	TestAccept/bundle/invariant/no_drift	🙈S	🙈S	🙈S	🙈S	🙈S	🙈S
🙈	TestAccept/bundle/resources/vector_search_endpoints/drift/recreated_same_name	🙈S	🙈S	🙈S	🙈S	🙈S	🙈S
🙈	TestAccept/bundle/resources/vector_search_indexes/recreate/embedding_dimension	🙈S	🙈S	🙈S	🙈S	🙈S	🙈S
🙈	TestAccept/ssh/connection	🙈S	🙈S	🙈S	🙈S	🙈S	🙈S
🔄	TestSyncIncrementalSyncFileToPythonNotebook	✅p	✅p	✅p	🔄f	✅p	✅p

Top 8 slowest tests (at least 2 minutes):

duration	env	testname
3:14	gcp windows	TestAccept
3:14	aws windows	TestAccept
3:09	azure windows	TestAccept
2:44	azure linux	TestFilerWorkspaceFilesExtensionsReadDir
2:43	gcp linux	TestAccept/bundle/resources/genie_spaces/recreate_when_gone/DATABRICKS_BUNDLE_ENGINE=direct
2:42	gcp windows	TestFilerRecursiveDelete/workspace_files
2:07	gcp windows	TestFilerWorkspaceFilesExtensionsRead
2:02	gcp linux	TestFilerWorkspaceFilesExtensionsDelete

denik

I think we should not have edge case for "internal" resources. We can just create resources.internal_files, populate it ourselves (forbid users from touching it) and then just let the framework handle it as a regular resource.

denik · 2026-07-29T08:56:20Z

        "deployments": [
          {
-            "command_path": "${workspace.snapshot_path}/files/src/main.py",
+            "command_path": "${internal.snapshot.full_path}/files/src/main.py",


Instead of custom path, can we keep using ${workspace.file_path}, just make it an alias internally to internal resource's snapshot path.

That way:

users know what this means

users can use the variable themselves and it'll work both in regular and immutable case.

${workspace.file_path} is ${internal.snapshot.full_path}/files and because we resolve the variables ${workspace.file_path} resolved to this pass. I'd prefer not to change the way variable reslution works and introduce some custom aliases

denik · 2026-08-03T13:35:31Z

@@ -35,12 +36,15 @@ type ACLEntry struct {
 // the same workspace location.
 // This interface exists so the implementation can later be replaced with a Go SDK call.


This does not explain why we need the interface - do we plan on supporting multiple implementation at the same time?

Strictly speaking we don't need it for SDK replacement later but we need this now in unit tests to replace the uploader with mock one

andrewnester · 2026-08-20T11:14:36Z

 All files and directories at the following location will be deleted: /Workspace/Users/[USERNAME]/.bundle/recreate-gone-genie-space-[UNIQUE_NAME]/default

-Destroy: 1 deleted
+Destroy: 0 deleted


Because of this fix: https://github.com/databricks/cli/pull/6084/changes#diff-26a7ca1d9267b02564c3de57565eea8310ba2099c4df658fb902af54a36662cdR172

Could have been a separate PR if unrelated to this work.

andrewnester · 2026-08-20T11:14:40Z

 All files and directories at the following location will be deleted: /Workspace/Users/[USERNAME]/.bundle/test-bundle/default

-Destroy: 1 deleted
+Destroy: 0 deleted


Because of this fix: https://github.com/databricks/cli/pull/6084/changes#diff-26a7ca1d9267b02564c3de57565eea8310ba2099c4df658fb902af54a36662cdR172

andrewnester · 2026-08-20T11:14:44Z

 All files and directories at the following location will be deleted: /Workspace/Users/[USERNAME]/.bundle/test-bundle/default

-Destroy: 1 deleted
+Destroy: 0 deleted


Because of this fix: https://github.com/databricks/cli/pull/6084/changes#diff-26a7ca1d9267b02564c3de57565eea8310ba2099c4df658fb902af54a36662cdR172

pietern · 2026-08-24T09:12:18Z

+// references resolved against the current bundle state. This is needed for immutable
+// folder bundles where source_code_path contains a reference to the snapshot's
+// full_path, which is only known after deploy.
+func (a *appRunner) resolvedSourceCodePath() (string, error) {


This routine is a point solution for getting the fully resolved deployment state.

It feels like this belongs on a generic function that returns the state. Then the source_code_path will be pre-resolved and we don't need the check above at all; the path is usable as is.

pietern · 2026-08-24T09:15:52Z

+		tag := resources.Type().Field(i).Tag.Get("bundle")
+		if tag == "internal" {
+			continue
+		}


The comment should explain why this is ignoring internal resources (I can't tell why).

pietern · 2026-08-24T09:16:55Z

 	// mutators need informed consent if they are potentially destructive.
-	bundle.ApplySeqContext(ctx, b,
+	bundle.ApplySeqContext(
+		ctx, b,


This and the above diffs are nops.

pietern · 2026-08-24T09:18:33Z

 	uploadPath := path.Join(b.Config.Workspace.ArtifactPath, InternalDirName)
 	uploadPath = ensureWorkspaceOrVolumesPrefix(uploadPath)
+	if b.IsImmutableFolder() {
+		uploadPath = path.Join("${resources.internal_immutable_snapshots.immutable.full_path}", "artifacts", InternalDirName)


The static variable ref should be a const.

pietern · 2026-08-24T09:19:22Z

-				if targetPath == "workspace.snapshot_path" {
+				if targetNode == "" {
 					continue
 				}


Is this empty check still needed?

pietern · 2026-08-24T09:20:30Z

-		// Skip fields configured in ignore_remote_changes.
-		if ignoreFilter.shouldIgnore(path.String()) {
-			return
-		}


Why was the move needed? Wondering if there is anything structural missing that makes the above code not work without the ignore check.

pietern · 2026-08-24T09:28:43Z

 	Secrets               map[string]*resources.Secret               `json:"secrets,omitempty"`
+
+	// Internal resources
+	Snapshots map[string]*resources.Snapshot `json:"internal_immutable_snapshots,omitempty" bundle:"internal"`


WDYT about prefixing this with a _ to make it clearer it is internal just by looking at the state.

pietern · 2026-08-24T09:30:02Z

 All files and directories at the following location will be deleted: /Workspace/Users/[USERNAME]/.bundle/recreate-gone-genie-space-[UNIQUE_NAME]/default

-Destroy: 1 deleted
+Destroy: 0 deleted


Could have been a separate PR if unrelated to this work.

pietern · 2026-08-24T12:45:42Z

+
 >>> [CLI] bundle deploy
-Uploading immutable bundle snapshot...
+Created internal_immutable_snapshots.immutable


This should show up differently in the deploy output.

pietern · 2026-08-24T12:51:58Z

 	artifactPath = ensureWorkspaceOrVolumesPrefix(artifactPath)
+	if b.IsImmutableFolder() {
+		artifactPath = path.Join("${resources.internal_immutable_snapshots.immutable.full_path}", "artifacts")
+	}


Cleanup is not done for immutable folders.

Separate note: should we write back the above artifactPath to workspace.artifact_path?

This is done already as a default in bundle/config/mutator/default_workspace_paths.go.

pietern · 2026-08-24T12:57:58Z

+		// Reads (dynamic): resources.internal_immutable_snapshots (and other internal keys)
+		// Warns and removes any internal resource fields set by the user.
+		mutator.RejectInternalResources(),
+


Nit: not a dynamic but a typed read.

pietern · 2026-08-24T13:00:57Z

+// into the in-memory state cache entry for the snapshot resource. This is needed when
+// deploying from a plan file: the plan JSON omits ZipContent (json:"-"), so InitForApply
+// leaves it empty, causing DoCreate to upload an empty zip and derive a wrong snapshot ID.
+func SyncZipContent(b *bundle.Bundle) {


Shouldn't this return an error on the !ok cases below?

pietern · 2026-08-24T13:02:13Z

+
+func (s *ResourceSnapshot) DoDelete(ctx context.Context, id string, state *SnapshotState) error {
+	return nil
+}


I believe the adapter can deal with methods that don't apply being unspecified.

denik · 2026-08-24T13:19:49Z

+type SnapshotState struct {
+	RemoteRoot   string              `json:"remote_root"`
+	RelativePath string              `json:"relative_path"`
+	FullPath     string              `json:"full_path"`


Are RelativePath and FullPath refer to remote location? Why don't they have Remote prefix like RemoteRoot?

Can you add comments and example values for these fields? It feels like there is some redundancy between RemoteRoot, RelativePath, FullPath but I cannot tell for sure because I don't know what these mean exactly.

internal: Refactor immutable folder upload as an internal resource in…

84608d6

… direct engine

andrewnester requested a review from denik July 28, 2026 11:29

andrewnester and others added 4 commits July 28, 2026 14:00

fix test and tidy up

a5ab287

updated the output

4a0d4bd

Merge branch 'main' into feat/upload-resource

c869f1d

fix lint

aa73e34

denik reviewed Jul 29, 2026

View reviewed changes

andrewnester added 6 commits July 31, 2026 17:00

make snapshot a normal resource

b4f272f

clean up

f31a466

fix test

5943979

fixes

195974d

refschema

b6bb79f

lint fixes

2b7b48f

andrewnester requested a review from denik August 3, 2026 13:00

denik reviewed Aug 3, 2026

View reviewed changes

andrewnester added 2 commits August 3, 2026 16:59

use HasInternalResources

54a7dc0

fix test and etc

fbd5229

andrewnester requested a review from denik August 4, 2026 09:11

andrewnester added 5 commits August 4, 2026 19:20

stable bundle id

7825722

fix tests

b1e0be1

fix typo

b8cdcec

update refschema

f32d7fc

remove deadcode + update test output

94eefc7

denik reviewed Aug 5, 2026

View reviewed changes

adr feedback

769c0c3

andrewnester requested a review from denik August 5, 2026 11:16

andrewnester added 2 commits August 5, 2026 13:57

refschema

c5cbf31

fix invariant coverage test

af7f3ab

andrewnester and others added 3 commits August 6, 2026 11:12

fix fmt

5a05041

Merge branch 'main' into feat/upload-resource

6c74b0b

fmt

e816436

pietern self-requested a review August 11, 2026 11:29

andrewnester and others added 7 commits August 13, 2026 11:04

Merge branch 'main' into feat/upload-resource

0c8c6b6

update out.test.toml

03d79bd

Merge branch 'main' into feat/upload-resource

d879631

fix build

ba0d5b8

fix lint

4ac3aed

fix output

d07f4f8

fix delete count

2408d9a

shreyas-goenka reviewed Aug 18, 2026

View reviewed changes

andrewnester and others added 3 commits August 19, 2026 16:58

fixes

8f2b500

Merge branch 'main' into feat/upload-resource

2060f2b

updated output

82f2714

andrewnester commented Aug 20, 2026

View reviewed changes

andrewnester requested a review from shreyas-goenka August 20, 2026 11:14

fix output

92ec65c

pietern reviewed Aug 24, 2026

View reviewed changes

denik reviewed Aug 24, 2026

View reviewed changes

build and store zip locally

b60469f

		@@ -35,12 +36,15 @@ type ACLEntry struct {
		// the same workspace location.
		// This interface exists so the implementation can later be replaced with a Go SDK call.

Uh oh!

Conversation

andrewnester commented Jul 28, 2026

Changes

Why

Tests

Uh oh!

github-actions Bot commented Jul 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Approval status: pending

/acceptance/bundle/ - needs approval

/bundle/ - needs approval

/cmd/bundle/ - needs approval

/libs/sync/ - needs approval

General files (require maintainer)

Uh oh!

eng-dev-ecosystem-bot commented Jul 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Integration test report

Uh oh!

denik left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

github-actions Bot commented Jul 28, 2026 •

edited

Loading

`/acceptance/bundle/` - needs approval

`/bundle/` - needs approval

`/cmd/bundle/` - needs approval

`/libs/sync/` - needs approval

eng-dev-ecosystem-bot commented Jul 28, 2026 •

edited

Loading

denik left a comment •

edited

Loading