Skip to content

Remove never-wired folder writability check - #6638

Open
denik wants to merge 1 commit into
mainfrom
denik/rm-folder-acl-writable
Open

Remove never-wired folder writability check#6638
denik wants to merge 1 commit into
mainfrom
denik/rm-folder-acl-writable

Conversation

@denik

@denik denik commented Sep 11, 2026

Copy link
Copy Markdown
Contributor

CheckWritable / FolderACL.CanWrite (with the Writability type and the isWriteLevel / userGroupNames helpers) were added in #5948 (Jul 2026) alongside the folder-ACL consolidation, but nothing ever called them. The live path, ResolveFolderACL, is used only by validate/folder_permissions.go for Permissions.Compare — never for the write check.

Since the whole write-check API is reachable only from its own unit tests, deadcode -test ./... doesn't flag it. ResolveFolderACL and its walk-up-to-ancestor test are kept.

This pull request and its description were written by Isaac.

CheckWritable/CanWrite (plus the Writability type and the isWriteLevel/
userGroupNames helpers) were added in #5948 alongside the folder ACL
consolidation, but no caller was ever wired up: the live path,
ResolveFolderACL, is used by validate/folder_permissions.go only for
Permissions.Compare, never for the write check.

Reachable only from its own unit tests, so deadcode -test never flagged it.
Keeps ResolveFolderACL and its walk-up-to-ancestor test.

Co-authored-by: Isaac <no-reply@databricks.com>
@github-actions

Copy link
Copy Markdown
Contributor

Waiting for approval

Based on git history, these people are best suited to review:

  • @pietern -- recent work in bundle/permissions/

Eligible reviewers: @andrewnester, @anton-107, @janniklasrose, @lennartkats-db, @shreyas-goenka

Suggestions based on git history. See OWNERS for ownership rules.

@eng-dev-ecosystem-bot

Copy link
Copy Markdown
Collaborator

Integration test report

Commit: 02988ba

Run: 34608137131

Env 🔄​flaky 💚​RECOVERED ✅​pass 🙈​skip Time
💚​ aws linux 1 275 15 5:14
💚​ aws windows 1 277 13 6:30
💚​ azure linux 1 274 15 5:54
🔄​ azure windows 4 1 272 13 10:52
💚​ gcp linux 1 275 15 6:12
💚​ gcp windows 1 277 13 6:42
Test Name aws linux aws windows azure linux azure windows gcp linux gcp windows
💚​ TestAccept 💚​R 💚​R 💚​R 💚​R 💚​R 💚​R
🔄​ TestFsCpDirToDirWithOverwriteFlag ✅​p ✅​p ✅​p 🔄​f ✅​p ✅​p
🔄​ TestFsCpDirToDirWithOverwriteFlag/dbfs_to_uc-volumes ✅​p ✅​p ✅​p 🔄​f ✅​p ✅​p
🔄​ TestFsRmDirRecursively ✅​p ✅​p ✅​p 🔄​f ✅​p ✅​p
🔄​ TestFsRmDirRecursively/uc-volumes ✅​p ✅​p ✅​p 🔄​f ✅​p ✅​p
Top 3 slowest tests (at least 2 minutes):
duration env testname
3:28 aws windows TestAccept
3:21 gcp windows TestAccept
3:18 azure windows TestAccept

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