Skip to content

bundle: preserve .designer.ipynb suffix when translating notebook task paths#5370

Open
artchen-db wants to merge 1 commit into
databricks:mainfrom
artchen-db:bundle/preserve-designer-ipynb-suffix
Open

bundle: preserve .designer.ipynb suffix when translating notebook task paths#5370
artchen-db wants to merge 1 commit into
databricks:mainfrom
artchen-db:bundle/preserve-designer-ipynb-suffix

Conversation

@artchen-db
Copy link
Copy Markdown

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:

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.goTestStripExtension covers .py / .ipynb / .r / .scala / .sql, the .designer.ipynb keep-case, and no-extension input.
  • bundle/config/mutator/translate_paths_test.goTestTranslatePathsDesignerNotebook (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.

…ask paths

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. `translateNotebookPath` unconditionally stripped `path.Ext(localRelPath)` — which is `.ipynb` for a `foo.designer.ipynb` path — leaving `foo.designer` and pointing the job's `notebook_path` at a file that does not exist.

Add `notebook.ExtensionDesigner` and a `StripExtension` helper that special-cases the designer suffix. Apply it in both branches of `translateNotebookPath` (the `skipLocalFileValidation` branch and the post-detection branch).

Reproduces with a bundle that has `tasks[].notebook_task.notebook_path: ./foo.designer.ipynb`: before this change the file synced as `foo.designer.ipynb` (correct, type `DESIGNER_FILE`) but the deployed job pointed at `foo.designer` (404). After this change the job points at `foo.designer.ipynb`.
@github-actions
Copy link
Copy Markdown
Contributor

An authorized user can trigger integration tests manually by following the instructions below:

Trigger:
go/deco-tests-run/cli

Inputs:

  • PR number: 5370
  • Commit SHA: d6a03e6e8ceef45d9d19c56cc8a1dd9e3a75ced2

Checks will be approved automatically on success.

@github-actions
Copy link
Copy Markdown
Contributor

Approval status: pending

/bundle/ - needs approval

Files: bundle/config/mutator/translate_paths.go, bundle/config/mutator/translate_paths_test.go
Suggested: @andrewnester
Also eligible: @pietern, @denik, @anton-107, @shreyas-goenka, @janniklasrose, @lennartkats-db

General files (require maintainer)

Files: libs/notebook/ext.go, libs/notebook/ext_test.go
Based on git history:

  • @andrewnester -- recent work in bundle/config/mutator/

Any maintainer (@andrewnester, @anton-107, @denik, @pietern, @shreyas-goenka, @simonfaltum, @renaudhartert-db) can approve all areas.
See OWNERS for ownership rules.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant