Skip to content

Commit 1054c19

Browse files
filer: restore INVALID_PARAMETER_VALUE branch (it does fire, with overwrite=true)
Previous commit (687c731) called the INVALID_PARAMETER_VALUE branch dead code based on conflict scenarios that didn't pass overwrite=true. Re-probing with overwrite=true (the mode bundle deploy uses) shows the server does return 400 INVALID_PARAMETER_VALUE for cross-type collisions, in two distinct message shapes: "Cannot overwrite the asset at X due to type mismatch (asked: ..., actual: ...)" "Requested node type [...] is different from the existing node type [...]" Both fire only when the existing object's node type differs from the upload (NOTEBOOK at /a/foo, regular-content overwrite-upload to /a/foo or /a/foo.py). Without overwrite=true, the same scenario surfaces as RESOURCE_ALREADY_EXISTS or ALREADY_EXISTS — which is why the no-overwrite probe missed this branch. Restore the mapping (returning fileAlreadyExistsError so the caller's errors.Is(err, fs.ErrExist) check still fires) and broaden the message pattern from "Requested node type" to also match "type mismatch", so both real-workspace messages are caught. Add unit tests for both shapes. Co-authored-by: Isaac
1 parent 687c731 commit 1054c19

2 files changed

Lines changed: 36 additions & 15 deletions

File tree

libs/filer/workspace_files_client.go

Lines changed: 22 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import (
1111
"net/http"
1212
"path"
1313
"slices"
14+
"strings"
1415
"time"
1516

1617
"github.com/databricks/databricks-sdk-go"
@@ -208,15 +209,30 @@ func (w *WorkspaceFilesClient) Write(ctx context.Context, name string, reader io
208209
return w.Write(ctx, name, bytes.NewReader(body), sliceWithout(mode, CreateParentDirectories)...)
209210
}
210211

211-
// Path already taken. /workspace/import returns 400 RESOURCE_ALREADY_EXISTS
212-
// for sequential conflicts and 409 ALREADY_EXISTS under concurrent contention
213-
// (observed in TestLock). Same-path collisions where the existing object is a
214-
// different node type (e.g. NOTEBOOK at /a/foo, upload regular content to
215-
// /a/foo) also surface here — verified against a real workspace, the server
216-
// returns one of the two already-exists codes regardless of the type mismatch.
212+
// Path already taken. /workspace/import returns this in three shapes,
213+
// verified against a real workspace:
214+
//
215+
// - 400 RESOURCE_ALREADY_EXISTS — sequential conflict, no overwrite flag.
216+
// - 409 ALREADY_EXISTS — concurrent contention (observed in TestLock).
217+
// - 400 INVALID_PARAMETER_VALUE with a "type mismatch" / "node type" message
218+
// — overwrite=true on a path where the existing object's node type differs
219+
// from the upload (e.g. NOTEBOOK at /a/foo, regular-content upload to
220+
// /a/foo with overwrite). The server refuses the overwrite even though the
221+
// caller asked for it; from the caller's perspective the path is occupied,
222+
// so we surface this as already-exists.
223+
//
224+
// Sources for the third bullet (`bundle deploy` issues with overwrite=true):
225+
// "Cannot overwrite the asset at X due to type mismatch (asked: ..., actual: ...)"
226+
// "Requested node type [...] is different from the existing node type [...]"
217227
if errors.Is(err, apierr.ErrResourceAlreadyExists) || errors.Is(err, apierr.ErrAlreadyExists) {
218228
return fileAlreadyExistsError{absPath}
219229
}
230+
if errors.Is(err, apierr.ErrInvalidParameterValue) {
231+
var aerr *apierr.APIError
232+
if errors.As(err, &aerr) && (strings.Contains(aerr.Message, "type mismatch") || strings.Contains(aerr.Message, "node type")) {
233+
return fileAlreadyExistsError{absPath}
234+
}
235+
}
220236

221237
// Caller has read access but no write access.
222238
if errors.Is(err, apierr.ErrPermissionDenied) {

libs/filer/workspace_files_client_test.go

Lines changed: 14 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -187,20 +187,25 @@ func TestWorkspaceFilesClientWriteErrorMapping(t *testing.T) {
187187
expectErrTarget: fileAlreadyExistsError{},
188188
},
189189
{
190-
// Verified against a real workspace: when an existing NOTEBOOK at /a/foo
191-
// (uploaded earlier as /a/foo.py with the source header) blocks a
192-
// regular-content upload to /a/foo, the server returns 409 ALREADY_EXISTS
193-
// rather than a node-type-specific code.
194-
name: "409 ALREADY_EXISTS for cross-type collision maps to fileAlreadyExistsError",
190+
name: "400 INVALID_PARAMETER_VALUE 'type mismatch' (overwrite=true) maps to fileAlreadyExistsError",
195191
apiErr: &apierr.APIError{
196-
StatusCode: http.StatusConflict,
197-
ErrorCode: "ALREADY_EXISTS",
198-
Message: "Node with name /dir/foo.py already exists. Please pass overwrite=true to update it.",
192+
StatusCode: http.StatusBadRequest,
193+
ErrorCode: "INVALID_PARAMETER_VALUE",
194+
Message: "Cannot overwrite the asset at /dir/foo due to type mismatch (asked: FILE, actual: NOTEBOOK).",
195+
},
196+
expectErrTarget: fileAlreadyExistsError{},
197+
},
198+
{
199+
name: "400 INVALID_PARAMETER_VALUE 'Requested node type' (overwrite=true) maps to fileAlreadyExistsError",
200+
apiErr: &apierr.APIError{
201+
StatusCode: http.StatusBadRequest,
202+
ErrorCode: "INVALID_PARAMETER_VALUE",
203+
Message: "Requested node type [FILE] is different from the existing node type [NOTEBOOK]",
199204
},
200205
expectErrTarget: fileAlreadyExistsError{},
201206
},
202207
{
203-
name: "400 INVALID_PARAMETER_VALUE passes through",
208+
name: "400 INVALID_PARAMETER_VALUE other message passes through",
204209
apiErr: &apierr.APIError{
205210
StatusCode: http.StatusBadRequest,
206211
ErrorCode: "INVALID_PARAMETER_VALUE",

0 commit comments

Comments
 (0)