fix: stop s3contents .s3keep 403s on read-only prefixes#148
Merged
Tianhao-Gu merged 1 commit intomainfrom Mar 27, 2026
Merged
Conversation
s3contents init() calls mkdir("") which PUTs a .s3keep file at the root
of each managed prefix. For tenant/group prefixes where users have
read-only access, this causes 403 errors that fill MinIO logs.
The previous patch wrapped mkdir in try/except, which caught the error
but still sent the PUT request (generating 403 log entries). This change
skips mkdir entirely since:
- MMS already creates .s3keep markers during user/group provisioning
- The genericmanager.py patch handles dirs without .s3keep via
DUMMY_CREATED_DATE fallback
Contributor
There was a problem hiding this comment.
Pull request overview
Updates the s3contents patching script used in the container image build to prevent init-time .s3keep writes that generate repeated 403s for read-only tenant/group prefixes, reducing MinIO log noise while relying on existing provisioning and the genericmanager fallback behavior.
Changes:
- Removes the
self.mkdir("")call froms3contentss3_fs.pyinit by patching it to a no-op (pass) instead of wrapping intry/except. - Expands the rationale in
patch_s3fs_init()docstring to explain why skipping.s3keepcreation is safe and desirable.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Coverage report
This PR does not seem to contain any modification to coverable code. |
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
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.
Summary
init()callsmkdir("")which PUTs.s3keepat each managed prefix root. For tenant/group prefixes where users have read-only access (e.g.tenant-sql-warehouse/nmdc/), this causes 403 errors that flood MinIO logs.mkdirin try/except — this caught the crash but the PUT still happened, generating 403 log entries on every S3ContentsManager init.mkdir("")withpassentirely, since MMS already creates.s3keepmarkers during provisioning, and thegenericmanager.pypatch handles directories without.s3keepviaDUMMY_CREATED_DATEfallback.Context
User agent
aiobotocore/3.3.0confirms these come from s3contents (Jupyter S3 file browser).Test plan
.s3keep403 entries appear in MinIO logs