Repository navigation
Add sandbox filesystem commands and filer - #7372
Draft
akshaysingla-db wants to merge 8 commits into
Draft
akshaysingla-db wants to merge 8 commits into
akshaysingla-db wants to merge 8 commits into
Conversation
akshaysingla-db
force-pushed
the
akshaysingla-db/stack/sandbox-files-filer
branch
from
November 5, 2026 05:46
4359bf9 to
47b4688
Compare
Collaborator
✅ Integration test reportCommit: c69e06e
Top 3 slowest tests (at least 2 minutes):
|
Models the server's semantics as observed on a live sandbox: uploads always replace and create parents, deletes of absent paths succeed, rename refuses an existing destination atomically, and most failures share one generic error with only the status differing. Unknown upload parameters such as overwrite are rejected, as on the real server. Co-authored-by: Isaac <no-reply@databricks.com>
The server rejects the overwrite upload parameter, so every write failed. Drop it and the hand-built HTTP client in favor of the typed SDK calls. Exclusive writes now upload to a temporary file and rename it into place, since rename refuses an existing destination atomically. Map the server's generic errors to the Filer contract by statting the path: reading a directory, listing a file, deleting a non-empty directory, and creating a directory over a file. Report deletes of absent paths and writes into missing parents as not found. Test the filer against the testserver fake instead of per-test stubs, which encoded server behavior that doesn't exist. Co-authored-by: Isaac <no-reply@databricks.com>
Enabled by TEST_SANDBOX_ID. Also add a suite for concurrent exclusive writes, run for the local and sandbox filers. Co-authored-by: Isaac <no-reply@databricks.com>
One test per command (cp, ls, cat, mkdir, rm), run against the testserver fake locally. With Cloud = true they also run against the existing sandbox named by TEST_SANDBOX_ID, so a run doesn't create a sandbox; cloud environments without one skip them. Co-authored-by: Isaac <no-reply@databricks.com>
- Map rename errors other than "already exists" like upload errors. - Give the exclusive-create temporary file a fixed-length name, so long target names stay within the filesystem's name limit, and stop claiming listings hide it. - Keep cp's description of directory copies in the sandbox help, and explain why the fs command clears its help group. - Share the sandbox cache key rule (profile, else host) in cacheProfile. - Show the effect of cp --overwrite, and ignore the downloaded directory. - Assert the full error for invalid sandbox paths. - Drop the changelog fragment: the sandbox command is still hidden. Co-authored-by: Isaac <no-reply@databricks.com>
- Report reading a symlink to a directory, and overwriting a directory, as "not a file" instead of the server's raw 409. - Stop the fake's mkdirAll at "/" so it terminates after a recursive delete of the root. - Show examples in each sandbox fs command's help, and say in cp's help that existing files are skipped without --overwrite. - Mark which comments rely on the API docs rather than observed behavior. - Capture --help as the first stanza of each sandbox fs acceptance test, and cover copying a file into an existing directory. Co-authored-by: Isaac <no-reply@databricks.com>
Contributor
|
Commit: Regression test reportSelected 5 changed tests (limit=20, 0 not selected) 5 tests: regression=5 unreleased=0 coverage=0 failing=0
TestAccept/cmd/sandbox/fs/cat: branch ✅ | main (dd0811b) ❌ | latest (v1.24.0) ❌main (dd0811b): latest (v1.24.0): same output as main (dd0811b) TestAccept/cmd/sandbox/fs/cp: branch ✅ | main (dd0811b) ❌ | latest (v1.24.0) ❌main (dd0811b): latest (v1.24.0): TestAccept/cmd/sandbox/fs/ls: branch ✅ | main (dd0811b) ❌ | latest (v1.24.0) ❌main (dd0811b): latest (v1.24.0): 2 more failure details omitted, see the full report in the run. |
This branch has not been deployed
This file contains hidden or 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
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.
🥞 Stacked PR
Use this link to review incremental changes.
Changes
Add
sandbox fs cp/ls/cat/mkdir/rmusing a sandbox filer and the existing filesystem commands. Paths useSANDBOX:/absolute/path, with cached sandbox names supported. Copies skip existing files unless--overwriteis set.Why
Sandbox file transfers need recursive copying and consistent filesystem commands. Uploads explicitly send
overwrite=falseby default and translate HTTP 412 into the filer's existing skip behavior. The pinned SDK predates this parameter, so upload uses its streaming HTTP client; other operations use generated methods. Requires server support for the overwrite parameter.Tests
Added command and filer regression coverage for recursive downloads, overwrite/skip, metadata pagination, path encoding, and workspace routing. Formatting, diff checks, and focused Go lint pass. Functional tests have not been run.
This PR was written by Codex.