diff --git a/NEXT_CHANGELOG.md b/NEXT_CHANGELOG.md index f28f0e6d257..2297f807b7d 100644 --- a/NEXT_CHANGELOG.md +++ b/NEXT_CHANGELOG.md @@ -9,5 +9,6 @@ ### Bundles * Retry transient HTTP 504 Gateway Timeout errors in direct deployment engine ([#5349](https://github.com/databricks/cli/pull/5349)). +* Preserve `.designer.ipynb` suffix when translating notebook task paths so Lakeflow Designer files referenced from a `notebook_task` resolve correctly in the workspace ([#5370](https://github.com/databricks/cli/pull/5370)). ### Dependency updates diff --git a/acceptance/bundle/paths/designer_notebook/databricks.yml b/acceptance/bundle/paths/designer_notebook/databricks.yml new file mode 100644 index 00000000000..291e63df797 --- /dev/null +++ b/acceptance/bundle/paths/designer_notebook/databricks.yml @@ -0,0 +1,14 @@ +bundle: + name: designer_paths + +resources: + jobs: + designer_job: + name: designer_job + tasks: + - task_key: designer_task + notebook_task: + notebook_path: ./src/test.designer.ipynb + - task_key: regular_task + notebook_task: + notebook_path: ./src/regular.ipynb diff --git a/acceptance/bundle/paths/designer_notebook/output.txt b/acceptance/bundle/paths/designer_notebook/output.txt new file mode 100644 index 00000000000..b473fac5f8d --- /dev/null +++ b/acceptance/bundle/paths/designer_notebook/output.txt @@ -0,0 +1,10 @@ +[ + { + "task_key": "designer_task", + "notebook_path": "/Workspace/Users/[USERNAME]/.bundle/designer_paths/default/files/src/test.designer.ipynb" + }, + { + "task_key": "regular_task", + "notebook_path": "/Workspace/Users/[USERNAME]/.bundle/designer_paths/default/files/src/regular" + } +] diff --git a/acceptance/bundle/paths/designer_notebook/script b/acceptance/bundle/paths/designer_notebook/script new file mode 100644 index 00000000000..6affc4a74eb --- /dev/null +++ b/acceptance/bundle/paths/designer_notebook/script @@ -0,0 +1 @@ +$CLI bundle validate -o json | jq '.resources.jobs.designer_job.tasks | map({task_key, notebook_path: .notebook_task.notebook_path})' diff --git a/acceptance/bundle/paths/designer_notebook/src/regular.ipynb b/acceptance/bundle/paths/designer_notebook/src/regular.ipynb new file mode 100644 index 00000000000..1ec2d26c287 --- /dev/null +++ b/acceptance/bundle/paths/designer_notebook/src/regular.ipynb @@ -0,0 +1,10 @@ +{ + "cells": [], + "metadata": { + "application/vnd.databricks.v1+notebook": { + "language": "python" + } + }, + "nbformat": 4, + "nbformat_minor": 0 +} diff --git a/acceptance/bundle/paths/designer_notebook/src/test.designer.ipynb b/acceptance/bundle/paths/designer_notebook/src/test.designer.ipynb new file mode 100644 index 00000000000..1ec2d26c287 --- /dev/null +++ b/acceptance/bundle/paths/designer_notebook/src/test.designer.ipynb @@ -0,0 +1,10 @@ +{ + "cells": [], + "metadata": { + "application/vnd.databricks.v1+notebook": { + "language": "python" + } + }, + "nbformat": 4, + "nbformat_minor": 0 +} diff --git a/bundle/config/mutator/translate_paths.go b/bundle/config/mutator/translate_paths.go index dc730c1ff1c..a0bb76e2e6e 100644 --- a/bundle/config/mutator/translate_paths.go +++ b/bundle/config/mutator/translate_paths.go @@ -196,8 +196,7 @@ func (t *translateContext) rewritePath( func (t *translateContext) translateNotebookPath(ctx context.Context, literal, localFullPath, localRelPath string) (string, error) { if t.skipLocalFileValidation { - localRelPathNoExt := strings.TrimSuffix(localRelPath, path.Ext(localRelPath)) - return path.Join(t.remoteRoot, localRelPathNoExt), nil + return path.Join(t.remoteRoot, notebook.StripExtension(localRelPath)), nil } nb, _, err := notebook.DetectWithFS(t.b.SyncRoot, localRelPath) @@ -229,9 +228,9 @@ to contain one of the following file extensions: [%s]`, literal, strings.Join(no return "", ErrIsNotNotebook{localFullPath} } - // Upon import, notebooks are stripped of their extension. - localRelPathNoExt := strings.TrimSuffix(localRelPath, path.Ext(localRelPath)) - return path.Join(t.remoteRoot, localRelPathNoExt), nil + // Upon import, notebooks are stripped of their extension. Designer files + // keep their full ".designer.ipynb" suffix. + return path.Join(t.remoteRoot, notebook.StripExtension(localRelPath)), nil } func (t *translateContext) translateFilePath(ctx context.Context, literal, localFullPath, localRelPath string) (string, error) { diff --git a/bundle/config/mutator/translate_paths_test.go b/bundle/config/mutator/translate_paths_test.go index bc2d1217909..226a848b723 100644 --- a/bundle/config/mutator/translate_paths_test.go +++ b/bundle/config/mutator/translate_paths_test.go @@ -31,6 +31,15 @@ func touchNotebookFile(t *testing.T, path string) { f.Close() } +// touchDesignerFile writes a minimal valid Lakeflow Designer notebook (Jupyter +// JSON) so that notebook.DetectWithFS recognizes it as a notebook. +func touchDesignerFile(t *testing.T, path string) { + err := os.MkdirAll(filepath.Dir(path), 0o700) + require.NoError(t, err) + contents := `{"cells":[],"metadata":{"application/vnd.databricks.v1+notebook":{"language":"python"}},"nbformat":4,"nbformat_minor":0}` + require.NoError(t, os.WriteFile(path, []byte(contents), 0o644)) +} + func touchEmptyFile(t *testing.T, path string) { err := os.MkdirAll(filepath.Dir(path), 0o700) require.NoError(t, err) @@ -1124,3 +1133,110 @@ func TestTranslatePathsWithSkipLocalFileValidationDirectory(t *testing.T) { // Directory path should be translated even though directory doesn't exist. assert.Equal(t, "/bundle/pipeline_root", b.Config.Resources.Pipelines["pipeline"].RootPath) } + +// TestTranslatePathsDesignerNotebook verifies that Lakeflow Designer notebooks +// (`*.designer.ipynb`) referenced by a notebook_task preserve their full +// suffix in the deployed notebook_path, since the workspace keeps that suffix +// on import (unlike regular `.ipynb`, which is stripped). +func TestTranslatePathsDesignerNotebook(t *testing.T) { + dir := t.TempDir() + touchDesignerFile(t, filepath.Join(dir, "src", "designer.designer.ipynb")) + touchNotebookFile(t, filepath.Join(dir, "src", "regular.py")) + + b := &bundle.Bundle{ + SyncRootPath: dir, + BundleRootPath: dir, + SyncRoot: vfs.MustNew(dir), + Config: config.Root{ + Workspace: config.Workspace{ + FilePath: "/bundle", + }, + Resources: config.Resources{ + Jobs: map[string]*resources.Job{ + "job": { + JobSettings: jobs.JobSettings{ + Tasks: []jobs.Task{ + { + NotebookTask: &jobs.NotebookTask{ + NotebookPath: "./src/designer.designer.ipynb", + }, + }, + { + NotebookTask: &jobs.NotebookTask{ + NotebookPath: "./src/regular.py", + }, + }, + }, + }, + }, + }, + }, + }, + } + + bundletest.SetLocation(b, ".", []dyn.Location{{File: filepath.Join(dir, "databricks.yml")}}) + + diags := bundle.ApplySeq(t.Context(), b, mutator.NormalizePaths(), mutator.TranslatePaths()) + require.NoError(t, diags.Error()) + + // Designer notebook keeps its full ".designer.ipynb" suffix. + assert.Equal(t, + "/bundle/src/designer.designer.ipynb", + b.Config.Resources.Jobs["job"].Tasks[0].NotebookTask.NotebookPath) + + // Regular notebook still has its extension stripped on import. + assert.Equal(t, + "/bundle/src/regular", + b.Config.Resources.Jobs["job"].Tasks[1].NotebookTask.NotebookPath) +} + +// TestTranslatePathsDesignerNotebookSkipLocalFileValidation verifies the +// designer-suffix preservation also holds on the config-remote-sync code path +// where the local file is not inspected. +func TestTranslatePathsDesignerNotebookSkipLocalFileValidation(t *testing.T) { + dir := t.TempDir() + + b := &bundle.Bundle{ + SyncRootPath: dir, + BundleRootPath: dir, + SyncRoot: vfs.MustNew(dir), + SkipLocalFileValidation: true, + Config: config.Root{ + Workspace: config.Workspace{ + FilePath: "/bundle", + }, + Resources: config.Resources{ + Jobs: map[string]*resources.Job{ + "job": { + JobSettings: jobs.JobSettings{ + Tasks: []jobs.Task{ + { + NotebookTask: &jobs.NotebookTask{ + NotebookPath: "./src/designer.designer.ipynb", + }, + }, + { + NotebookTask: &jobs.NotebookTask{ + NotebookPath: "./src/regular.ipynb", + }, + }, + }, + }, + }, + }, + }, + }, + } + + bundletest.SetLocation(b, ".", []dyn.Location{{File: filepath.Join(dir, "databricks.yml")}}) + + diags := bundle.ApplySeq(t.Context(), b, mutator.NormalizePaths(), mutator.TranslatePaths()) + require.NoError(t, diags.Error()) + + assert.Equal(t, + "/bundle/src/designer.designer.ipynb", + b.Config.Resources.Jobs["job"].Tasks[0].NotebookTask.NotebookPath) + assert.Equal(t, + "/bundle/src/regular", + b.Config.Resources.Jobs["job"].Tasks[1].NotebookTask.NotebookPath) +} diff --git a/libs/notebook/ext.go b/libs/notebook/ext.go index bbffe23c2d2..49155aaa8eb 100644 --- a/libs/notebook/ext.go +++ b/libs/notebook/ext.go @@ -1,6 +1,11 @@ package notebook -import "github.com/databricks/databricks-sdk-go/service/workspace" +import ( + "path" + "strings" + + "github.com/databricks/databricks-sdk-go/service/workspace" +) const ( ExtensionNone string = "" @@ -9,8 +14,22 @@ const ( ExtensionScala string = ".scala" ExtensionSql string = ".sql" ExtensionJupyter string = ".ipynb" + // ExtensionDesigner is the compound suffix for Lakeflow Designer files. + // Unlike other notebook types, designer files keep this full suffix when + // imported into the workspace. + ExtensionDesigner string = ".designer.ipynb" ) +// StripExtension returns the workspace path for a local notebook file. +// Designer files keep their full ".designer.ipynb" suffix in the workspace; +// other notebook types lose their extension on import. +func StripExtension(name string) string { + if strings.HasSuffix(name, ExtensionDesigner) { + return name + } + return strings.TrimSuffix(name, path.Ext(name)) +} + // Extensions lists all notebook file extensions. var Extensions = []string{ ExtensionPython, diff --git a/libs/notebook/ext_test.go b/libs/notebook/ext_test.go new file mode 100644 index 00000000000..c1eb47ccd20 --- /dev/null +++ b/libs/notebook/ext_test.go @@ -0,0 +1,32 @@ +package notebook + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestStripExtension(t *testing.T) { + for _, tc := range []struct { + in string + want string + }{ + {"foo.py", "foo"}, + {"foo.ipynb", "foo"}, + {"foo.r", "foo"}, + {"foo.scala", "foo"}, + {"foo.sql", "foo"}, + {"a/b/c.ipynb", "a/b/c"}, + + // Designer files keep their full ".designer.ipynb" suffix. + {"foo.designer.ipynb", "foo.designer.ipynb"}, + {"a/b/c.designer.ipynb", "a/b/c.designer.ipynb"}, + + // Files without a known extension are passed through path.Ext; + // the last-segment extension is removed. + {"foo", "foo"}, + {"foo.unknown", "foo"}, + } { + assert.Equal(t, tc.want, StripExtension(tc.in), "input=%q", tc.in) + } +}