From f27980bebec78c13c5226d453f5f431aa4c17c0e Mon Sep 17 00:00:00 2001 From: simonfaltum Date: Tue, 9 Jun 2026 22:30:52 +0200 Subject: [PATCH 1/2] Fix trampoline notebook filename collision Wrapper notebooks were named notebook__. Since both keys can contain underscores, distinct pairs like (a_b, c) and (a, b_c) mapped to the same filename and one task silently ran the other task's wrapper. Prefix the job key length to make the name unique per pair. Co-authored-by: Isaac --- .../trampoline/conditional_transform_test.go | 2 +- bundle/trampoline/trampoline.go | 6 +- bundle/trampoline/trampoline_test.go | 78 +++++++++++++++++-- 3 files changed, 77 insertions(+), 9 deletions(-) diff --git a/bundle/trampoline/conditional_transform_test.go b/bundle/trampoline/conditional_transform_test.go index 97cfa94df08..928ed578d17 100644 --- a/bundle/trampoline/conditional_transform_test.go +++ b/bundle/trampoline/conditional_transform_test.go @@ -108,7 +108,7 @@ func TestTransformWithExperimentalSettingSetToTrue(t *testing.T) { task := b.Config.Resources.Jobs["job1"].Tasks[0] require.Nil(t, task.PythonWheelTask) require.NotNil(t, task.NotebookTask) - require.Equal(t, "/Workspace/files/my_bundle/.databricks/bundle/development/.internal/notebook_job1_key1", task.NotebookTask.NotebookPath) + require.Equal(t, "/Workspace/files/my_bundle/.databricks/bundle/development/.internal/notebook_4_job1_key1", task.NotebookTask.NotebookPath) require.Len(t, task.Libraries, 1) require.Equal(t, "/Workspace/Users/test@test.com/bundle/dist/test.jar", task.Libraries[0].Jar) diff --git a/bundle/trampoline/trampoline.go b/bundle/trampoline/trampoline.go index 600ce3d9c64..9c779e45c13 100644 --- a/bundle/trampoline/trampoline.go +++ b/bundle/trampoline/trampoline.go @@ -59,7 +59,11 @@ func (m *trampoline) generateNotebookWrapper(ctx context.Context, b *bundle.Bund return err } - notebookName := fmt.Sprintf("notebook_%s_%s", task.JobKey, task.Task.TaskKey) + // Job and task keys may themselves contain underscores, so joining them with + // "_" alone is ambiguous: "a_b"+"c" and "a"+"b_c" would map to the same file + // and one task would silently run the other's wrapper. Prefixing the job key + // length makes the name unique per (job key, task key) pair. + notebookName := fmt.Sprintf("notebook_%d_%s_%s", len(task.JobKey), task.JobKey, task.Task.TaskKey) localNotebookPath := filepath.Join(internalDir, notebookName+".py") err = os.MkdirAll(filepath.Dir(localNotebookPath), 0o755) diff --git a/bundle/trampoline/trampoline_test.go b/bundle/trampoline/trampoline_test.go index 53b1e3811a6..62dd8a48202 100644 --- a/bundle/trampoline/trampoline_test.go +++ b/bundle/trampoline/trampoline_test.go @@ -17,11 +17,13 @@ type functions struct{} func (f *functions) GetTasks(b *bundle.Bundle) []TaskWithJobKey { var tasks []TaskWithJobKey - for k := range b.Config.Resources.Jobs["test"].Tasks { - tasks = append(tasks, TaskWithJobKey{ - JobKey: "test", - Task: &b.Config.Resources.Jobs["test"].Tasks[k], - }) + for k, job := range b.Config.Resources.Jobs { + for i := range job.Tasks { + tasks = append(tasks, TaskWithJobKey{ + JobKey: k, + Task: &job.Tasks[i], + }) + } } return tasks @@ -85,7 +87,7 @@ func TestGenerateTrampoline(t *testing.T) { dir, err := b.InternalDir(ctx) require.NoError(t, err) - filename := filepath.Join(dir, "notebook_test_to_trampoline.py") + filename := filepath.Join(dir, "notebook_4_test_to_trampoline.py") bytes, err := os.ReadFile(filename) require.NoError(t, err) @@ -93,6 +95,68 @@ func TestGenerateTrampoline(t *testing.T) { require.Equal(t, "Hello from Trampoline", string(bytes)) task := b.Config.Resources.Jobs["test"].Tasks[0] - require.Equal(t, "/Workspace/files/my_bundle/.databricks/bundle/development/.internal/notebook_test_to_trampoline", task.NotebookTask.NotebookPath) + require.Equal(t, "/Workspace/files/my_bundle/.databricks/bundle/development/.internal/notebook_4_test_to_trampoline", task.NotebookTask.NotebookPath) require.Nil(t, task.PythonWheelTask) } + +func TestGenerateTrampolineWithCollidingKeys(t *testing.T) { + tmpDir := t.TempDir() + + newJob := func(taskKey string) *resources.Job { + return &resources.Job{ + JobSettings: jobs.JobSettings{ + Tasks: []jobs.Task{ + { + TaskKey: taskKey, + PythonWheelTask: &jobs.PythonWheelTask{ + PackageName: "test", + EntryPoint: "run", + }, + }, + }, + }, + } + } + + b := &bundle.Bundle{ + BundleRootPath: filepath.Join(tmpDir, "parent", "my_bundle"), + SyncRootPath: filepath.Join(tmpDir, "parent"), + Config: config.Root{ + Workspace: config.Workspace{ + FilePath: "/Workspace/files", + }, + Bundle: config.Bundle{ + Target: "development", + }, + Resources: config.Resources{ + // Without the length prefix both pairs would produce "notebook_a_b_c" + // and the second wrapper would overwrite the first. + Jobs: map[string]*resources.Job{ + "a_b": newJob("c"), + "a": newJob("b_c"), + }, + }, + }, + } + ctx := t.Context() + + funcs := functions{} + trampoline := NewTrampoline("test_trampoline", &funcs, "Hello from {{.MyName}}") + diags := bundle.Apply(ctx, b, trampoline) + require.NoError(t, diags.Error()) + + dir, err := b.InternalDir(ctx) + require.NoError(t, err) + + for _, name := range []string{"notebook_3_a_b_c", "notebook_1_a_b_c"} { + _, err := os.Stat(filepath.Join(dir, name+".py")) + require.NoError(t, err) + } + + require.Equal(t, + "/Workspace/files/my_bundle/.databricks/bundle/development/.internal/notebook_3_a_b_c", + b.Config.Resources.Jobs["a_b"].Tasks[0].NotebookTask.NotebookPath) + require.Equal(t, + "/Workspace/files/my_bundle/.databricks/bundle/development/.internal/notebook_1_a_b_c", + b.Config.Resources.Jobs["a"].Tasks[0].NotebookTask.NotebookPath) +} From 953c623995a6e12e8b68ae7f2c8c294313069034 Mon Sep 17 00:00:00 2001 From: simonfaltum Date: Wed, 10 Jun 2026 00:21:12 +0200 Subject: [PATCH 2/2] Remove redundant comments Co-authored-by: Isaac --- bundle/trampoline/trampoline.go | 6 ++---- bundle/trampoline/trampoline_test.go | 2 -- 2 files changed, 2 insertions(+), 6 deletions(-) diff --git a/bundle/trampoline/trampoline.go b/bundle/trampoline/trampoline.go index 9c779e45c13..b3683688fee 100644 --- a/bundle/trampoline/trampoline.go +++ b/bundle/trampoline/trampoline.go @@ -59,10 +59,8 @@ func (m *trampoline) generateNotebookWrapper(ctx context.Context, b *bundle.Bund return err } - // Job and task keys may themselves contain underscores, so joining them with - // "_" alone is ambiguous: "a_b"+"c" and "a"+"b_c" would map to the same file - // and one task would silently run the other's wrapper. Prefixing the job key - // length makes the name unique per (job key, task key) pair. + // Keys may contain underscores, so joining with "_" alone is ambiguous ("a_b"+"c" + // vs "a"+"b_c"); the job key length prefix keeps the filename unique per task. notebookName := fmt.Sprintf("notebook_%d_%s_%s", len(task.JobKey), task.JobKey, task.Task.TaskKey) localNotebookPath := filepath.Join(internalDir, notebookName+".py") diff --git a/bundle/trampoline/trampoline_test.go b/bundle/trampoline/trampoline_test.go index 62dd8a48202..08a55c5a0f1 100644 --- a/bundle/trampoline/trampoline_test.go +++ b/bundle/trampoline/trampoline_test.go @@ -129,8 +129,6 @@ func TestGenerateTrampolineWithCollidingKeys(t *testing.T) { Target: "development", }, Resources: config.Resources{ - // Without the length prefix both pairs would produce "notebook_a_b_c" - // and the second wrapper would overwrite the first. Jobs: map[string]*resources.Job{ "a_b": newJob("c"), "a": newJob("b_c"),