Skip to content

Commit 65d9bf9

Browse files
authored
Preserve .designer.ipynb suffix when translating notebook task paths (#5370)
## Summary Lakeflow Designer files keep their full `.designer.ipynb` suffix when imported into the workspace, unlike regular notebooks (`.py`, `.ipynb`, `.r`, `.scala`, `.sql`) which lose their extension on import. `translateNotebookPath` in `bundle/config/mutator/translate_paths.go` unconditionally stripped `path.Ext(localRelPath)`. For a `foo.designer.ipynb` path, `path.Ext` returns `.ipynb`, leaving `foo.designer` — the deployed job's `notebook_path` then points at a file that does not exist. ## Fix Add `notebook.ExtensionDesigner = ".designer.ipynb"` and a `StripExtension` helper in `libs/notebook/ext.go` that special-cases the designer suffix. Apply it in both branches of `translateNotebookPath` (the `skipLocalFileValidation` branch and the post-detection branch). ## Reproduction A bundle with: ```yaml resources: jobs: designer_job: tasks: - task_key: t notebook_task: notebook_path: ./foo.designer.ipynb ``` | | Before | After | |---|---|---| | File in workspace | `foo.designer.ipynb` (`DESIGNER_FILE`) ✓ | `foo.designer.ipynb` (`DESIGNER_FILE`) ✓ | | Deployed `notebook_path` | `foo.designer` → 404 ✗ | `foo.designer.ipynb` ✓ | ## Tests - `libs/notebook/ext_test.go` — `TestStripExtension` covers `.py / .ipynb / .r / .scala / .sql`, the `.designer.ipynb` keep-case, and no-extension input. - `bundle/config/mutator/translate_paths_test.go` — `TestTranslatePathsDesignerNotebook` (file present, mixed designer + regular tasks) and `TestTranslatePathsDesignerNotebookSkipLocalFileValidation` (config-remote-sync branch). Both assert designer suffix is preserved and `.py` / `.ipynb` are still stripped. ## Out of scope Other call sites strip `path.Ext` in the same pattern (`libs/sync/snapshot_state.go`, `bundle/generate/downloader.go`, `cmd/workspace/workspace/import_dir.go`, `libs/filer/workspace_files_extensions_client.go`). The job-task path covers the reported user-visible bug; whether designer files need the same special-case in sync / generate / import-dir flows depends on whether they round-trip through those paths in a way the workspace API doesn't already handle. Happy to extend in a follow-up if reviewers want.
1 parent 9af226f commit 65d9bf9

10 files changed

Lines changed: 218 additions & 6 deletions

File tree

NEXT_CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,5 +9,6 @@
99

1010
### Bundles
1111
* Retry transient HTTP 504 Gateway Timeout errors in direct deployment engine ([#5349](https://github.com/databricks/cli/pull/5349)).
12+
* 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)).
1213

1314
### Dependency updates
Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
bundle:
2+
name: designer_paths
3+
4+
resources:
5+
jobs:
6+
designer_job:
7+
name: designer_job
8+
tasks:
9+
- task_key: designer_task
10+
notebook_task:
11+
notebook_path: ./src/test.designer.ipynb
12+
- task_key: regular_task
13+
notebook_task:
14+
notebook_path: ./src/regular.ipynb
Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
[
2+
{
3+
"task_key": "designer_task",
4+
"notebook_path": "/Workspace/Users/[USERNAME]/.bundle/designer_paths/default/files/src/test.designer.ipynb"
5+
},
6+
{
7+
"task_key": "regular_task",
8+
"notebook_path": "/Workspace/Users/[USERNAME]/.bundle/designer_paths/default/files/src/regular"
9+
}
10+
]
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
$CLI bundle validate -o json | jq '.resources.jobs.designer_job.tasks | map({task_key, notebook_path: .notebook_task.notebook_path})'
Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
{
2+
"cells": [],
3+
"metadata": {
4+
"application/vnd.databricks.v1+notebook": {
5+
"language": "python"
6+
}
7+
},
8+
"nbformat": 4,
9+
"nbformat_minor": 0
10+
}
Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
{
2+
"cells": [],
3+
"metadata": {
4+
"application/vnd.databricks.v1+notebook": {
5+
"language": "python"
6+
}
7+
},
8+
"nbformat": 4,
9+
"nbformat_minor": 0
10+
}

bundle/config/mutator/translate_paths.go

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -196,8 +196,7 @@ func (t *translateContext) rewritePath(
196196

197197
func (t *translateContext) translateNotebookPath(ctx context.Context, literal, localFullPath, localRelPath string) (string, error) {
198198
if t.skipLocalFileValidation {
199-
localRelPathNoExt := strings.TrimSuffix(localRelPath, path.Ext(localRelPath))
200-
return path.Join(t.remoteRoot, localRelPathNoExt), nil
199+
return path.Join(t.remoteRoot, notebook.StripExtension(localRelPath)), nil
201200
}
202201

203202
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
229228
return "", ErrIsNotNotebook{localFullPath}
230229
}
231230

232-
// Upon import, notebooks are stripped of their extension.
233-
localRelPathNoExt := strings.TrimSuffix(localRelPath, path.Ext(localRelPath))
234-
return path.Join(t.remoteRoot, localRelPathNoExt), nil
231+
// Upon import, notebooks are stripped of their extension. Designer files
232+
// keep their full ".designer.ipynb" suffix.
233+
return path.Join(t.remoteRoot, notebook.StripExtension(localRelPath)), nil
235234
}
236235

237236
func (t *translateContext) translateFilePath(ctx context.Context, literal, localFullPath, localRelPath string) (string, error) {

bundle/config/mutator/translate_paths_test.go

Lines changed: 116 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,15 @@ func touchNotebookFile(t *testing.T, path string) {
3131
f.Close()
3232
}
3333

34+
// touchDesignerFile writes a minimal valid Lakeflow Designer notebook (Jupyter
35+
// JSON) so that notebook.DetectWithFS recognizes it as a notebook.
36+
func touchDesignerFile(t *testing.T, path string) {
37+
err := os.MkdirAll(filepath.Dir(path), 0o700)
38+
require.NoError(t, err)
39+
contents := `{"cells":[],"metadata":{"application/vnd.databricks.v1+notebook":{"language":"python"}},"nbformat":4,"nbformat_minor":0}`
40+
require.NoError(t, os.WriteFile(path, []byte(contents), 0o644))
41+
}
42+
3443
func touchEmptyFile(t *testing.T, path string) {
3544
err := os.MkdirAll(filepath.Dir(path), 0o700)
3645
require.NoError(t, err)
@@ -1124,3 +1133,110 @@ func TestTranslatePathsWithSkipLocalFileValidationDirectory(t *testing.T) {
11241133
// Directory path should be translated even though directory doesn't exist.
11251134
assert.Equal(t, "/bundle/pipeline_root", b.Config.Resources.Pipelines["pipeline"].RootPath)
11261135
}
1136+
1137+
// TestTranslatePathsDesignerNotebook verifies that Lakeflow Designer notebooks
1138+
// (`*.designer.ipynb`) referenced by a notebook_task preserve their full
1139+
// suffix in the deployed notebook_path, since the workspace keeps that suffix
1140+
// on import (unlike regular `.ipynb`, which is stripped).
1141+
func TestTranslatePathsDesignerNotebook(t *testing.T) {
1142+
dir := t.TempDir()
1143+
touchDesignerFile(t, filepath.Join(dir, "src", "designer.designer.ipynb"))
1144+
touchNotebookFile(t, filepath.Join(dir, "src", "regular.py"))
1145+
1146+
b := &bundle.Bundle{
1147+
SyncRootPath: dir,
1148+
BundleRootPath: dir,
1149+
SyncRoot: vfs.MustNew(dir),
1150+
Config: config.Root{
1151+
Workspace: config.Workspace{
1152+
FilePath: "/bundle",
1153+
},
1154+
Resources: config.Resources{
1155+
Jobs: map[string]*resources.Job{
1156+
"job": {
1157+
JobSettings: jobs.JobSettings{
1158+
Tasks: []jobs.Task{
1159+
{
1160+
NotebookTask: &jobs.NotebookTask{
1161+
NotebookPath: "./src/designer.designer.ipynb",
1162+
},
1163+
},
1164+
{
1165+
NotebookTask: &jobs.NotebookTask{
1166+
NotebookPath: "./src/regular.py",
1167+
},
1168+
},
1169+
},
1170+
},
1171+
},
1172+
},
1173+
},
1174+
},
1175+
}
1176+
1177+
bundletest.SetLocation(b, ".", []dyn.Location{{File: filepath.Join(dir, "databricks.yml")}})
1178+
1179+
diags := bundle.ApplySeq(t.Context(), b, mutator.NormalizePaths(), mutator.TranslatePaths())
1180+
require.NoError(t, diags.Error())
1181+
1182+
// Designer notebook keeps its full ".designer.ipynb" suffix.
1183+
assert.Equal(t,
1184+
"/bundle/src/designer.designer.ipynb",
1185+
b.Config.Resources.Jobs["job"].Tasks[0].NotebookTask.NotebookPath)
1186+
1187+
// Regular notebook still has its extension stripped on import.
1188+
assert.Equal(t,
1189+
"/bundle/src/regular",
1190+
b.Config.Resources.Jobs["job"].Tasks[1].NotebookTask.NotebookPath)
1191+
}
1192+
1193+
// TestTranslatePathsDesignerNotebookSkipLocalFileValidation verifies the
1194+
// designer-suffix preservation also holds on the config-remote-sync code path
1195+
// where the local file is not inspected.
1196+
func TestTranslatePathsDesignerNotebookSkipLocalFileValidation(t *testing.T) {
1197+
dir := t.TempDir()
1198+
1199+
b := &bundle.Bundle{
1200+
SyncRootPath: dir,
1201+
BundleRootPath: dir,
1202+
SyncRoot: vfs.MustNew(dir),
1203+
SkipLocalFileValidation: true,
1204+
Config: config.Root{
1205+
Workspace: config.Workspace{
1206+
FilePath: "/bundle",
1207+
},
1208+
Resources: config.Resources{
1209+
Jobs: map[string]*resources.Job{
1210+
"job": {
1211+
JobSettings: jobs.JobSettings{
1212+
Tasks: []jobs.Task{
1213+
{
1214+
NotebookTask: &jobs.NotebookTask{
1215+
NotebookPath: "./src/designer.designer.ipynb",
1216+
},
1217+
},
1218+
{
1219+
NotebookTask: &jobs.NotebookTask{
1220+
NotebookPath: "./src/regular.ipynb",
1221+
},
1222+
},
1223+
},
1224+
},
1225+
},
1226+
},
1227+
},
1228+
},
1229+
}
1230+
1231+
bundletest.SetLocation(b, ".", []dyn.Location{{File: filepath.Join(dir, "databricks.yml")}})
1232+
1233+
diags := bundle.ApplySeq(t.Context(), b, mutator.NormalizePaths(), mutator.TranslatePaths())
1234+
require.NoError(t, diags.Error())
1235+
1236+
assert.Equal(t,
1237+
"/bundle/src/designer.designer.ipynb",
1238+
b.Config.Resources.Jobs["job"].Tasks[0].NotebookTask.NotebookPath)
1239+
assert.Equal(t,
1240+
"/bundle/src/regular",
1241+
b.Config.Resources.Jobs["job"].Tasks[1].NotebookTask.NotebookPath)
1242+
}

libs/notebook/ext.go

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,11 @@
11
package notebook
22

3-
import "github.com/databricks/databricks-sdk-go/service/workspace"
3+
import (
4+
"path"
5+
"strings"
6+
7+
"github.com/databricks/databricks-sdk-go/service/workspace"
8+
)
49

510
const (
611
ExtensionNone string = ""
@@ -9,8 +14,22 @@ const (
914
ExtensionScala string = ".scala"
1015
ExtensionSql string = ".sql"
1116
ExtensionJupyter string = ".ipynb"
17+
// ExtensionDesigner is the compound suffix for Lakeflow Designer files.
18+
// Unlike other notebook types, designer files keep this full suffix when
19+
// imported into the workspace.
20+
ExtensionDesigner string = ".designer.ipynb"
1221
)
1322

23+
// StripExtension returns the workspace path for a local notebook file.
24+
// Designer files keep their full ".designer.ipynb" suffix in the workspace;
25+
// other notebook types lose their extension on import.
26+
func StripExtension(name string) string {
27+
if strings.HasSuffix(name, ExtensionDesigner) {
28+
return name
29+
}
30+
return strings.TrimSuffix(name, path.Ext(name))
31+
}
32+
1433
// Extensions lists all notebook file extensions.
1534
var Extensions = []string{
1635
ExtensionPython,

libs/notebook/ext_test.go

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
package notebook
2+
3+
import (
4+
"testing"
5+
6+
"github.com/stretchr/testify/assert"
7+
)
8+
9+
func TestStripExtension(t *testing.T) {
10+
for _, tc := range []struct {
11+
in string
12+
want string
13+
}{
14+
{"foo.py", "foo"},
15+
{"foo.ipynb", "foo"},
16+
{"foo.r", "foo"},
17+
{"foo.scala", "foo"},
18+
{"foo.sql", "foo"},
19+
{"a/b/c.ipynb", "a/b/c"},
20+
21+
// Designer files keep their full ".designer.ipynb" suffix.
22+
{"foo.designer.ipynb", "foo.designer.ipynb"},
23+
{"a/b/c.designer.ipynb", "a/b/c.designer.ipynb"},
24+
25+
// Files without a known extension are passed through path.Ext;
26+
// the last-segment extension is removed.
27+
{"foo", "foo"},
28+
{"foo.unknown", "foo"},
29+
} {
30+
assert.Equal(t, tc.want, StripExtension(tc.in), "input=%q", tc.in)
31+
}
32+
}

0 commit comments

Comments
 (0)