Skip to content

[WIP] reimplement UC Volumes filer on the files/v2 client#6018

Open
renaudhartert-db wants to merge 1 commit into
mainfrom
fs-files-v2
Open

[WIP] reimplement UC Volumes filer on the files/v2 client#6018
renaudhartert-db wants to merge 1 commit into
mainfrom
fs-files-v2

Conversation

@renaudhartert-db

Copy link
Copy Markdown
Contributor

What

The fs command's UC Volumes filer now runs on the new sdk-go/files/v2 client instead of hand-written Files API calls. This filer is shared, so bundle library uploads to Volumes go through the same path.

files/v2 is not published to the Databricks Go proxy yet, so it is vendored under libs/tmp/ together with the two unpublished sibling modules it needs (auth and options). The already-published sdk-go/core module stays a normal dependency and is not copied. libs/tmp/README.md records where the copy came from and how to remove it once the module is published (rewrite the import prefix back and delete the directory).

files/v2 ships its own multipart/resumable upload engine, so the CLI's separate libs/upload engine is removed. The DATABRICKS_EXPERIMENTAL_MULTIPART_UPLOAD flag still gates whether large Volumes uploads are split into parts; it now selects multipart-vs-single-shot within files/v2 rather than switching engines. Default behavior (single-shot PUT) is unchanged.

Why

Groundwork for the CLI to consume sdk-go/files/v2 as a real dependency once it is published. Vendoring lets us build and dogfood the client, and its own upload engine replaces the CLI-side one so there is a single implementation to maintain.

Notable in the diff

  • libs/tmp/ is a verbatim vendored copy. The only edits are the import-path rewrite and two fixes to the generated client (empty response bodies and HTTP method casing); both are upstream generator bugs and are listed in libs/tmp/README.md. It is excluded from lint/format in .golangci.yaml so it stays byte-for-byte with upstream.
  • libs/filer/files_client.go is rewritten onto *files.Client. NewFilesClient gains a ctx parameter (threaded through its callers).
  • Shared code: the Volumes filer is used by both fs and bundle/libraries, so bundle Volumes uploads are affected. .github/OWNERS retargets the removed /libs/upload/ entry to /libs/tmp/. go.mod/NOTICE gain sdk-go/core as a direct dependency.

Out of scope

  • Local, DBFS, and workspace filers are unchanged; files/v2 only backs the UC Volumes path.
  • Publishing sdk-go/files/v2 is separate work; libs/tmp/ is the interim copy.

This pull request and its description were written by Isaac.

@github-actions

github-actions Bot commented Jul 22, 2026

Copy link
Copy Markdown
Contributor

Approval status: pending

/bundle/ - needs approval

Files: bundle/libraries/filer_volume.go
Suggested: @pietern
Also eligible: @denik, @shreyas-goenka, @janniklasrose, @andrewnester, @anton-107, @lennartkats-db

/cmd/fs/ - needs approval

4 files changed
Suggested: @simonfaltum
Also eligible: @Divyansh-db, @hectorcast-db, @parthban-db, @tanmay-db, @tejaskochar-db, @mihaimitrea-db, @chrisst, @rauchy

/experimental/air/ - needs approval

Files: experimental/air/cmd/snapshot.go
Suggested: @riddhibhagwat-db
Also eligible: @apeforest, @bfontain, @lu-wang-dl, @panchalhp-db, @vinchenzo-db, @maggiewang-db, @ben-hansen-db, @pardis-beikzadeh-db

/integration/ - needs approval

Files: integration/cmd/fs/helpers_test.go, integration/libs/filer/helpers_test.go
Suggested: @simonfaltum
Also eligible: @Divyansh-db, @hectorcast-db, @parthban-db, @tanmay-db, @tejaskochar-db, @mihaimitrea-db, @chrisst, @rauchy

/internal/ - needs approval

Files: internal/build/notice_test.go
Suggested: @simonfaltum
Also eligible: @Divyansh-db, @hectorcast-db, @parthban-db, @tanmay-db, @tejaskochar-db, @mihaimitrea-db, @chrisst, @rauchy

/libs/filer/ - needs approval

Files: libs/filer/files_client.go, libs/filer/files_client_auth.go, libs/filer/files_client_test.go
Suggested: @simonfaltum
Also eligible: @Divyansh-db, @hectorcast-db, @parthban-db, @tanmay-db, @tejaskochar-db, @mihaimitrea-db, @chrisst, @rauchy

General files (require maintainer)

44 files changed
Based on git history:

  • @pietern -- recent work in ./, libs/filer/, libs/testserver/

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

@eng-dev-ecosystem-bot

eng-dev-ecosystem-bot commented Jul 22, 2026

Copy link
Copy Markdown
Collaborator

Integration test report

Commit: 39fe347

Run: 29899233743

Env 🟨​KNOWN 🔄​flaky 💚​RECOVERED 🙈​SKIP ✅​pass 🙈​skip Time
💚​ aws linux 4 4 314 1052 5:10
💚​ aws windows 4 4 316 1050 7:31
🔄​ azure linux 1 4 4 313 1051 5:26
💚​ azure windows 4 4 316 1049 7:33
🟨​ gcp linux 3 1 4 313 1053 8:45
🟨​ gcp windows 3 1 4 315 1051 13:20
9 interesting tests: 4 SKIP, 3 KNOWN, 1 RECOVERED, 1 flaky
Test Name aws linux aws windows azure linux azure windows gcp linux gcp windows
💚​ TestAccept 💚​R 💚​R 💚​R 💚​R 💚​R 💚​R
🙈​ TestAccept/bundle/invariant/no_drift 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S
🙈​ TestAccept/bundle/resources/vector_search_endpoints/drift/recreated_same_name 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S
🙈​ TestAccept/bundle/resources/vector_search_indexes/recreate/embedding_dimension 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S
🙈​ TestAccept/ssh/connection 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S
🔄​ TestClustersGet ✅​p ✅​p 🔄​f ✅​p ✅​p ✅​p
🟨​ TestFetchRepositoryInfoAPI_FromRepo 💚​R 💚​R 💚​R 💚​R 🟨​K 🟨​K
🟨​ TestFetchRepositoryInfoAPI_FromRepo/root 💚​R 💚​R 💚​R 💚​R 🟨​K 🟨​K
🟨​ TestFetchRepositoryInfoAPI_FromRepo/subdir 💚​R 💚​R 💚​R 💚​R 🟨​K 🟨​K
Top 6 slowest tests (at least 2 minutes):
duration env testname
8:43 gcp windows TestAccept
6:25 azure windows TestAccept
6:15 aws windows TestAccept
3:01 aws linux TestAccept
2:52 azure linux TestAccept
2:51 gcp linux TestAccept

@renaudhartert-db renaudhartert-db changed the title fs: reimplement UC Volumes filer on the files/v2 client [WIP] reimplement UC Volumes filer on the files/v2 client Jul 22, 2026
Reworks the `fs` command's UC Volumes filer (also used by bundle library
uploads) to run on the new `sdk-go/files/v2` client instead of hand-rolled
Files API calls plus a separate CLI-side multipart upload engine.

`files/v2` is not published to the Go proxy yet, so it is vendored under
`libs/tmp/` along with the two unpublished sibling modules it needs (`auth`
and `options`); the published `sdk-go/core` module stays a normal dependency.
The only edits to the vendored copy are the import-path rewrite and two fixes
to the generated client (see below). `libs/tmp/README.md` records the copy and
the swap-and-delete steps for when the module is published.

The filer now delegates upload, download, listing, mkdir, delete, and stat to
`files/v2`, which brings its own multipart/resumable engine. The CLI's own
`libs/upload` engine is therefore removed. `DATABRICKS_EXPERIMENTAL_MULTIPART_UPLOAD`
still gates whether large Volumes uploads are split into parts, but now selects
multipart-vs-single-shot within files/v2 rather than a separate engine.

This pull request and its description were written by Isaac.

Co-authored-by: Isaac
@renaudhartert-db

Copy link
Copy Markdown
Contributor Author

Pushed fixes for the two red checks on the first CI run:

Integration test — TestFsMkdirWhenFileExistsAtPath/uc-volumes (real, PR-caused): fs mkdir over an existing file must surface as fs.ErrExist. The Files API reports this as HTTP 409, which the SDK's apierr.Code maps to codes.Aborted (not codes.AlreadyExists), so my codes-based checks missed it. Switched all filer error mapping from apierr.Code(err) == codes.X to matching the HTTP status directly (via a httpStatus helper), which is what the pre-existing implementation did. This also hardens the not-found mappings, which had the same latent mismatch. Modelled the "409 on create-directory-over-a-file" behavior in libs/testserver (it previously created the directory unconditionally, which is why no acceptance/unit test caught this) and added a Mkdir unit test.

lint — deadcode: the deadcode check (separate from golangci-lint) flagged four unreachable exported functions in the vendored libs/tmp copy (WithPartSize, WithAccountID, WithProfileFile, WithProfile). Added libs/tmp/ to the exclusion list in tools/check_deadcode.py, consistent with keeping the vendored copy byte-for-byte with upstream.

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.

2 participants