Add `filer.Filer` to read notebooks from WSFS without omitting their extension by shreyas-goenka · Pull Request #1457 · databricks/cli

shreyas-goenka · 2024-05-27T15:09:22Z

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.

… stat

andrewnester · 2024-05-30T09:37:52Z

+	}
+
+	// Not the correct language. Return early.
+	if stat.Language != extensionsToLanguages[ext] {


Out of curiosity, how can this happen?

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.

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)).

) ## 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>

…extension (#1457) ## 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. --------- Co-authored-by: Pieter Noordhuis <pieter.noordhuis@databricks.com>

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)).

) ## 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>

shreyas-goenka added 11 commits May 27, 2024 17:08

wip
0229dd3

do not rely on fuse mount
add2dd1

cleanup some todos and add test for dup name error
fdacdcc

more modular extension business
6ab53d5

rename file
49a0b49

add test for language check
42e1bb5

better main documentation
b55c00e

test for directories + notebook clash
f80d338

clear todos
c75b5e2

-
a38295c

better error messagE
b514498

shreyas-goenka marked this pull request as ready for review May 28, 2024 12:46

shreyas-goenka requested review from andrewnester and pietern May 28, 2024 12:46

shreyas-goenka changed the title ~~Add filer for the workspace FUSE mount~~Add filer to emulate a workspace filesystem FUSEMay 28, 2024

shreyas-goenka changed the title ~~Add filer to emulate a workspace filesystem FUSE~~Add filer.Filer to emulate a workspace filesystem FUSEMay 28, 2024

Merge remote-tracking branch 'origin' into wsfs-filer
0fb56da

pietern reviewed May 28, 2024
View reviewed changes

shreyas-goenka added 5 commits May 29, 2024 11:32

remove unnecessary sort
deb2107

make multiline
747ec5b

address more comments
c9a72c5

-
8db2bd7

address comments and fix the ipynb information loss:
1324bba

shreyas-goenka changed the title ~~Add filer.Filer to emulate a workspace filesystem FUSE~~Add a filer to read files and notebooks with their extensions includedMay 29, 2024

shreyas-goenka changed the title ~~Add a filer to read files and notebooks with their extensions included~~Add filer.Filer to read notebooks from WSFS without omitting their extensionMay 29, 2024

shreyas-goenkaand others added 5 commits May 30, 2024 00:49

do not use error directive
4fec905

final cleanup
4fe597f

Break out function for stat call
628a592

Return error if we expect a notebook and it isn't; never return a nil…
8bd6449
… stat

Capitalize
72538cf

pietern added 4 commits May 30, 2024 10:52

Move test assertions around; capitalize
0b5d9f5

Download Jupyter notebook as IPYNB
45786de

Rename to be similar to workspace files client
6ca5bce

Fix Jupyter contents check
5af3855

andrewnester reviewed May 30, 2024
View reviewed changes

pietern added 4 commits May 30, 2024 13:39

Address comments
d219f13

Bubble up errors from stat calls
dbeee88

filer -> wsfs
55eaa2d

Fix comment
55662ce

pietern requested a review from andrewnester May 30, 2024 11:52

andrewnester approved these changes May 30, 2024
View reviewed changes

pietern enabled auto-merge May 30, 2024 11:57

pietern added this pull request to the merge queue May 30, 2024

Merged via the queue into main with commit ec33a7cMay 30, 2024

pietern deleted the wsfs-filer branch May 30, 2024 12:06

pietern mentioned this pull request May 31, 2024
Databricks Bundle Deploy fails from within Databricks for databricks notebooks. #1263
Closed

pietern mentioned this pull request Jun 4, 2024
Release v0.221.0 #1470
Merged

pietern mentioned this pull request Jul 3, 2024
Replace vfs.Path with extension-aware filer when running on DBR #1556
Merged

Navigation Menu

Uh oh!

Add `filer.Filer` to read notebooks from WSFS without omitting their extension - #1457

Add `filer.Filer` to read notebooks from WSFS without omitting their extension#1457
pietern merged 30 commits into
mainfrom
wsfs-filer

shreyas-goenka commented May 27, 2024•
edited
Loading

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

andrewnesterMay 30, 2024

Uh oh!

pieternMay 30, 2024

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

shreyas-goenka commented May 27, 2024• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Changes

Tests

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

andrewnesterMay 30, 2024

Choose a reason for hiding this comment

Uh oh!

pieternMay 30, 2024

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

shreyas-goenka commented May 27, 2024•
edited
Loading