filer: switch workspace upload from import-file to /workspace/import - #6149
Draft
Sankalp-Mittal wants to merge 9 commits into
Draft
filer: switch workspace upload from import-file to /workspace/import#6149Sankalp-Mittal wants to merge 9 commits into
Sankalp-Mittal wants to merge 9 commits into
Conversation
Replace POST /api/2.0/workspace-files/import-file/{path} with the multipart
variant of POST /api/2.0/workspace/import (via the SDK's Workspace.Upload +
format=AUTO). The previous endpoint is deprecated; the new one has a higher
rate limit (30 vs 20 rps/workspace) and is ~1.5-2x faster for typical bundle
deployments.
Error handling is rewritten to branch on SDK sentinels (errors.Is against
ErrNotFound / ErrResourceAlreadyExists / ErrAlreadyExists /
ErrInvalidParameterValue / ErrPermissionDenied) and the AIP-193 ErrorInfo
reason WORKSPACE_OBJECT_TYPE_MISMATCH, with a message-substring fallback for
workspaces where WP-6031 has not rolled out.
The testserver now decodes the multipart /workspace/import body and mirrors the
real endpoint's format=AUTO notebook detection. A new bodyContains option on the
fault mechanism lets a test target a single file's upload, which is no longer
possible by URL since every upload shares the /workspace/import path.
Co-authored-by: Isaac
Collaborator
Integration test reportCommit: 9a67c01
21 interesting tests: 11 flaky, 4 SKIP, 3 RECOVERED, 2 FAIL, 1 KNOWN
Top 13 slowest tests (at least 2 minutes):
|
Sankalp-Mittal
marked this pull request as ready for review
August 4, 2026 08:22
Contributor
Approval status: pending
|
Sankalp-Mittal
marked this pull request as draft
August 4, 2026 11:10
The size limits in WorkspaceFilesClient.Write were described using internal config flag names and figures measured by hand against one workspace, neither of which a reader can verify. Replace them with the documented limits and link the sources. This corrects one figure: the notebook cap was described as a flat 10 MiB, but IPYNB notebooks are documented at 100 MB. Only source-format notebooks are capped at 10 MB, so the limit depends on how format=AUTO classifies the payload. Also restore the rate limit doc link to MaxRequestsInFlight, which the comment carried when the constant was introduced in #81. The value is unchanged. Add a comment above each test in workspace_files_client_test.go stating what it covers. Co-authored-by: Isaac
The local_code_source test filtered recorded uploads by URL path, which no longer matches: /workspace/import carries the target filename in the multipart body. Filter on the body instead, the same way auto-migrate-empty-tfstate does. --del-field raw_body is dropped because the tarball is binary and the request recorder already summarizes it as a size placeholder. This test was added in #6110, after the upload migration branch was cut, so it was not covered by the earlier fixture updates. Co-authored-by: Isaac
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
2 participants
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
Replace
POST /api/2.0/workspace-files/import-file/{path}with the multipartvariant of
POST /api/2.0/workspace/import(via the SDK'sWorkspace.Uploadwith
format=AUTO) as the transport for all bundle file uploads. This is asingle call site:
WorkspaceFilesClient.Writeinlibs/filer.The error handling in
Writeis rewritten to branch on SDK sentinels(
errors.IsagainstErrNotFound,ErrResourceAlreadyExists,ErrAlreadyExists,ErrInvalidParameterValue,ErrPermissionDenied) insteadof raw HTTP status codes and message-substring matching. The overwrite/type-
mismatch collision is detected via the AIP-193
ErrorInforeasonWORKSPACE_OBJECT_TYPE_MISMATCH, with a message-substring fallback forworkspaces where that detail (WP-6031) has not yet rolled out.
Why
/workspace-files/import-fileis deprecated./workspace/importhas a documented rate limit of 30 requests/sec perworkspace (API rate limits), higher than the limit that applied to
the previous endpoint, and is ~1.5–2× faster for a typical bundle deploy.
path collisions.
Why multipart
/workspace/importhas two request forms. The JSON form sends contentbase64-encoded in a
contentfield capped at 10 MB, returningMAX_NOTEBOOK_SIZE_EXCEEDEDabove it (see thecontentfield description forworkspace.Importin.codegen/cli.json). The multipart form posts the bytes asa file part and is bounded only by the workspace file size limit, so it accepts
the payload sizes
import-filedid.Because
format=AUTOlets the server classify each payload, the applicablelimit depends on the classification rather than on our request:
format=AUTO.py/.sql/.scala/.rwith aDatabricks notebook sourceheader)The notebook limits are enforced by the workspace for both
/workspace/importand
/workspace-files/import-file, so migrating between the two does not changethe maximum uploadable notebook size.
Testserver
libs/testservernow decodes the multipart/workspace/importbody andmirrors the real endpoint's
format=AUTOnotebook detection for.py/.sql/.scala/.r(header comment → NOTEBOOK, otherwise FILE), andreturns the collision error shape the CLI branches on.
acceptance/internal/prepare_server.gorecords multipart bodies in anormalized, deterministic form (
multipart_formwith sorted fields; large orbinary parts summarized) so recorded requests stay reviewable.
bodyContainsmatch(
fault.py --body-contains SUBSTR). Because every upload now shares the/workspace/importmethod+path, a single file's upload can only be targetedby its multipart
pathfield; the two migrate/destroy fault tests rely onthis.
Tests
bundle/sync-upload-edge-casescovers the cases thatdiffer between the two endpoints: a 12 MiB binary (over the 10 MB JSON cap), an empty
file, notebooks in three languages, a header-less
.py(stored as FILE), a.lvdash.jsondashboard, and non-ASCII / spaced filenames. It asserts eachupload's object type/language and that every upload sets
format=AUTO.libs/filer/workspace_files_client_test.gocover the successpath (format/overwrite) and every error mapping (already-exists shapes,
type-mismatch via ErrorInfo reason, 403, 500).
libs/testserver/fault_test.gocovers the newbodyContainsmatch.jqassertions in affected scripts re-aimed from
.pathto.body.multipart_form.path.Notes
is confirmed rolled out everywhere, and can be removed afterward.
This pull request and its description were written by Isaac.