Add filer.Filer to read notebooks from WSFS without omitting their extension
#1457
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
shreyas-goenka
changed the title
shreyas-goenka
changed the title
filer.Filer to emulate a workspace filesystem FUSE
pietern
reviewed
May 28, 2024
shreyas-goenka
changed the title
filer.Filer to emulate a workspace filesystem FUSE
shreyas-goenka
changed the title
filer.Filer to read notebooks from WSFS without omitting their extension
| } | ||
|
|
||
| // Not the correct language. Return early. | ||
| if stat.Language != extensionsToLanguages[ext] { |
There was a problem hiding this comment.
Out of curiosity, how can this happen?
There was a problem hiding this comment.
Let's say we're trying to stat foo.scala. Without extension this is foo. It can be a Python notebook and the language returned in the stat is PYTHON. Then this check will return false, because we can't (and shouldn't) access foo.scala if it is a Python notebook, only if it is a Scala notebook.
andrewnester
approved these changes
May 30, 2024
pietern
added a commit
that referenced
this pull request
Jun 4, 2024
CLI: * Update OpenAPI spec ([#1466](#1466)). Bundles: * Upgrade TF provider to 1.46.0 ([#1460](#1460)). * Add support for Lakehouse monitoring ([#1307](#1307)). * Make dbt-sql and default-sql templates public ([#1463](#1463)). Internal: * Abstract over filesystem interaction with libs/vfs ([#1452](#1452)). * Add `filer.Filer` to read notebooks from WSFS without omitting their extension ([#1457](#1457)). * Fix listing notebooks in a subdirectory ([#1468](#1468)). API Changes: * Changed `databricks account storage-credentials list` command to return . * Added `databricks consumer-listings batch-get` command. * Added `databricks consumer-providers batch-get` command. * Removed `databricks apps create-deployment` command. * Added `databricks apps deploy` command. OpenAPI commit 37b925eba37dfb3d7e05b6ba2d458454ce62d3a0 (2024-06-03) Dependency updates: * Bump github.com/hashicorp/go-version from 1.6.0 to 1.7.0 ([#1454](#1454)). * Bump github.com/hashicorp/hc-install from 0.6.4 to 0.7.0 ([#1453](#1453)).
Merged
github-merge-queue bot
pushed a commit
that referenced
this pull request
Jun 4, 2024
CLI: * Update OpenAPI spec ([#1466](#1466)). Bundles: * Upgrade TF provider to 1.46.0 ([#1460](#1460)). * Add support for Lakehouse monitoring ([#1307](#1307)). * Make dbt-sql and default-sql templates public ([#1463](#1463)). Internal: * Abstract over filesystem interaction with libs/vfs ([#1452](#1452)). * Add `filer.Filer` to read notebooks from WSFS without omitting their extension ([#1457](#1457)). * Fix listing notebooks in a subdirectory ([#1468](#1468)). API Changes: * Changed `databricks account storage-credentials list` command to return . * Added `databricks consumer-listings batch-get` command. * Added `databricks consumer-providers batch-get` command. * Removed `databricks apps create-deployment` command. * Added `databricks apps deploy` command. OpenAPI commit 37b925eba37dfb3d7e05b6ba2d458454ce62d3a0 (2024-06-03) Dependency updates: * Bump github.com/hashicorp/go-version from 1.6.0 to 1.7.0 ([#1454](#1454)). * Bump github.com/hashicorp/hc-install from 0.6.4 to 0.7.0 ([#1453](#1453)).
github-merge-queue bot
pushed a commit
that referenced
this pull request
Jul 3, 2024
) ## Changes The FUSE mount of the workspace file system on DBR doesn't include file extensions for notebooks. When these notebooks are checked into a repository, they do have an extension. PR #1457 added a filer type that is aware of this disparity and makes these notebooks show up as if they do have these extensions. This change swaps out the native `vfs.Path` with one that uses this filer when running on DBR. Follow up: consolidate between interfaces exported by `filer.Filer` and `vfs.Path`. ## Tests * Unit tests pass * (Manually ran a snapshot build on DBR against a bundle with notebooks) --------- Co-authored-by: Andrew Nester <andrew.nester@databricks.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Changes
This PR adds a filer that'll allow us to read notebooks from the WSFS using their full paths (with the extension included). The filer relies on the existing workspace filer (and consequently the workspace import/export/list APIs).
Using this filer along with a virtual filesystem layer (https://github.com/databricks/cli/pull/1452/files) will allow us to use our custom implementation (which preserves the notebook extensions) rather than the default mount available via DBR when the CLI is run from DBR.
Tests
Integration tests.