Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Extend "notebook not found" error to warn about missing extension #1920

Merged
merged 5 commits into from
Nov 21, 2024
Merged

Extend "notebook not found" error to warn about missing extension #1920

merged 5 commits into from
Nov 21, 2024

Conversation

shreyas-goenka
Copy link
Contributor

@shreyas-goenka shreyas-goenka commented Nov 20, 2024 •
edited
Loading

Changes

The full workspace path for a notebook does not contain the notebook's extension. If a user converts that file path to a relative path (like /Workspace/bundle_root/bar/nb -> ./bar/nb), they can be confused as to why the new file path does not work.

The changes in this PR nudge them to add the appropriate file extension (e.g., ./bar/nb.py or ./bar/nb.ipynb). We

One common way users can end up in this scenario is by using the view job as YAML functionality in the Databricks UI.

Tests

Unit test and manually.

(.venv) ➜  bundle-playground git:(master) ✗ cli bundle validate 
Error: notebook ./foo not found. Local notebook references are expected
to contain one of the following file extensions: [.py, .r, .scala, .sql, .ipynb]

When we detect that a candidate file exists then the error message is more targeted:

➜  bundle-playground git:(master) ✗ cli bundle validate
Error: notebook ./foo not found. Did you mean ./foo.py?
Local notebook references are expected to contain one of the following
file extensions: [.py, .r, .scala, .sql, .ipynb]

@shreyas-goenka shreyas-goenka changed the title Extend notebook not found error to warn about missing file extesion Extend notebook not found error to warn about missing extensions Nov 20, 2024
@shreyas-goenka shreyas-goenka marked this pull request as ready for review November 20, 2024 17:10
pietern
pietern reviewed Nov 20, 2024
View reviewed changes
Copy link
Contributor

@pietern pietern left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice, thanks. One suggestion in the comments.

bundle/config/mutator/translate_paths.go Show resolved Hide resolved
@github-actions GitHub Actions
Copy link

If integration tests don't run automatically, an authorized user can run them manually by following the instructions below:

Trigger:
go/deco-tests-run/cli

Inputs:

  • PR number: 1920
  • Commit SHA: 46f828b0c82f889c910205ab64f62a7d43386f14

Checks will be approved automatically on success.

@eng-dev-ecosystem-bot
Copy link
Collaborator

Test Details: go/deco-tests/11943918301

@pietern pietern changed the title Extend notebook not found error to warn about missing extensions Extend "notebook not found" error to warn about missing extension Nov 21, 2024
pietern
pietern approved these changes Nov 21, 2024
View reviewed changes
Copy link
Contributor

@pietern pietern left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, LGTM!

Please update the PR description to include the "did you mean" bit before merging.

@shreyas-goenka shreyas-goenka merged commit c2e2abc into main Nov 21, 2024
9 of 10 checks passed
@shreyas-goenka shreyas-goenka deleted the warn/missing-ext branch November 21, 2024 10:51
pietern added a commit that referenced this pull request Dec 5, 2024
@pietern
[Release] Release v0.236.0
b570470 
**New features for Databricks Asset Bundles:**

This release adds support for managing Unity Catalog volumes as part of your bundle configuration.

Bundles:
 * Add DABs support for Unity Catalog volumes ([#1762](#1762)).
 * Support lookup by name of notification destinations ([#1922](#1922)).
 * Extend "notebook not found" error to warn about missing extension ([#1920](#1920)).
 * Skip sync warning if no sync paths are defined ([#1926](#1926)).
 * Add validation for single node clusters ([#1909](#1909)).
 * Fix segfault in bundle summary command ([#1937](#1937)).
 * Add the `bundle_uuid` helper function for templates ([#1947](#1947)).
 * Add default value for `volume_type` for DABs ([#1952](#1952)).
 * Properly read Git metadata when running inside workspace ([#1945](#1945)).
 * Upgrade TF provider to 1.59.0 ([#1960](#1960)).

Internal:
 * Breakout variable lookup into separate files and tests ([#1921](#1921)).
 * Add golangci-lint v1.62.2 ([#1953](#1953)).

Dependency updates:
 * Bump golang.org/x/term from 0.25.0 to 0.26.0 ([#1907](#1907)).
 * Bump github.com/Masterminds/semver/v3 from 3.3.0 to 3.3.1 ([#1930](#1930)).
 * Bump github.com/stretchr/testify from 1.9.0 to 1.10.0 ([#1932](#1932)).
 * Bump github.com/databricks/databricks-sdk-go from 0.51.0 to 0.52.0 ([#1931](#1931)).
@pietern pietern mentioned this pull request Dec 5, 2024
Merged
github-merge-queue bot pushed a commit that referenced this pull request Dec 5, 2024
@pietern
[Release] Release v0.236.0 (#1966)
7ffe93e 
**New features for Databricks Asset Bundles:**

This release adds support for managing Unity Catalog volumes as part of
your bundle configuration.

Bundles:
* Add DABs support for Unity Catalog volumes
([#1762](#1762)).
* Support lookup by name of notification destinations
([#1922](#1922)).
* Extend "notebook not found" error to warn about missing extension
([#1920](#1920)).
* Skip sync warning if no sync paths are defined
([#1926](#1926)).
* Add validation for single node clusters
([#1909](#1909)).
* Fix segfault in bundle summary command
([#1937](#1937)).
* Add the `bundle_uuid` helper function for templates
([#1947](#1947)).
* Add default value for `volume_type` for DABs
([#1952](#1952)).
* Properly read Git metadata when running inside workspace
([#1945](#1945)).
* Upgrade TF provider to 1.59.0
([#1960](#1960)).

Internal:
* Breakout variable lookup into separate files and tests
([#1921](#1921)).
* Add golangci-lint v1.62.2
([#1953](#1953)).

Dependency updates:
* Bump golang.org/x/term from 0.25.0 to 0.26.0
([#1907](#1907)).
* Bump github.com/Masterminds/semver/v3 from 3.3.0 to 3.3.1
([#1930](#1930)).
* Bump github.com/stretchr/testify from 1.9.0 to 1.10.0
([#1932](#1932)).
* Bump github.com/databricks/databricks-sdk-go from 0.51.0 to 0.52.0
([#1931](#1931)).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@pietern pietern pietern approved these changes

@andrewnester andrewnester Awaiting requested review from andrewnester

@denik denik Awaiting requested review from denik

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@shreyas-goenka @eng-dev-ecosystem-bot @pietern