Skip to content

Commit 5fa9c5f

Browse files
filer: detect notebook already-exists across both error formats (#5106)
## Summary The Workspace files import-file API used to return `Path (<path>) already exists.` for notebook conflicts. The format has changed on some workspaces to `RESOURCE_ALREADY_EXISTS: <path> already exists. ...`. The original regex no longer matched the new format, so `fs.ErrExist` was not returned — breaking `TestImportDirDoesNotOverwrite` and 8 `TestFilerWorkspaceNotebook` subtests on every cloud. The new format might not have been rolled out to all workspaces yet (see [databricks-sdk-go#1639](databricks/databricks-sdk-go#1639)), and the JSON `error_code` is empty in both. Both messages end with `already exists.`, so we anchor on that substring and return the request `absPath` in the error rather than parsing the message. The integration test assertion is updated to expect the path with extension (`tc.name`) instead of without (`tc.nameWithoutExt`), since `absPath` is the request path. ## Test plan - [x] `go test ./libs/filer/...` passes locally - [x] `TestFilerWorkspaceNotebook` passes on aws-prod-ucws (new format) - [ ] Verify a workspace still emitting the old format also passes (best-effort — single substring match handles both) This pull request was AI-assisted by Isaac.
1 parent 5d9e6e4 commit 5fa9c5f

2 files changed

Lines changed: 8 additions & 12 deletions

File tree

integration/libs/filer/filer_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -443,7 +443,7 @@ func TestFilerWorkspaceNotebook(t *testing.T) {
443443
// Assert uploading a second time fails due to overwrite mode missing
444444
err = f.Write(ctx, tc.name, strings.NewReader(tc.content2))
445445
require.ErrorIs(t, err, fs.ErrExist)
446-
assert.Regexp(t, `file already exists: .*/`+tc.nameWithoutExt+`$`, err.Error())
446+
assert.Regexp(t, `file already exists: .*/`+tc.name+`$`, err.Error())
447447

448448
// Try uploading the notebook again with overwrite flag. This time it should succeed.
449449
err = f.Write(ctx, tc.name, strings.NewReader(tc.content2), filer.OverwriteIfExists)

libs/filer/workspace_files_client.go

Lines changed: 7 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,6 @@ import (
1111
"net/http"
1212
"net/url"
1313
"path"
14-
"regexp"
1514
"slices"
1615
"strings"
1716
"time"
@@ -209,16 +208,13 @@ func (w *WorkspaceFilesClient) Write(ctx context.Context, name string, reader io
209208
return fileAlreadyExistsError{absPath}
210209
}
211210

212-
// This API returns 400 if the file already exists, when the object type is notebook
213-
regex := regexp.MustCompile(`Path \((.*)\) already exists.`)
214-
if aerr.StatusCode == http.StatusBadRequest && regex.MatchString(aerr.Message) {
215-
// Parse file path from regex capture group
216-
matches := regex.FindStringSubmatch(aerr.Message)
217-
if len(matches) == 2 {
218-
return fileAlreadyExistsError{matches[1]}
219-
}
220-
221-
// Default to path specified to filer.Write if regex capture fails
211+
// This API returns 400 if the file already exists when the object type is notebook.
212+
// Both the historical "Path (<path>) already exists." format and the newer
213+
// "RESOURCE_ALREADY_EXISTS: <path> already exists. ..." format end with the same
214+
// "already exists." marker; the JSON error_code is empty in both. The new format
215+
// might not have been rolled out to all workspaces yet, so we anchor on the shared
216+
// marker and return absPath rather than parsing the message.
217+
if aerr.StatusCode == http.StatusBadRequest && strings.Contains(aerr.Message, "already exists.") {
222218
return fileAlreadyExistsError{absPath}
223219
}
224220

0 commit comments

Comments
 (0)