Is BUCKET_PREFIX supposed to work? #190

Activity

1. TimPietruskyRunPod commented on Jun 2, 2026

   

   TimPietruskyRunPod

   on Jun 2, 2026

   Contributor

   More actions

   @jpswelch's PR #191 implements this — when `BUCKET_PREFIX` is set, uploads go to `{prefix}/{job_id}/{original_filename}`; otherwise behavior is unchanged. I've reviewed the code, asked for a changeset entry, and it should be ready to merge once that's added.

   If you want to test it before it lands, the image will be available at `runpod/worker-comfyui:feature-bucket-prefix-base` once PR #223 (per-PR CI builds) merges and triggers a build on #191.
2. TimPietruskyRunPod

   added

   P3Low-risk cleanup, docs, polish, ergonomics, or speculative feature.Low-risk cleanup, docs, polish, ergonomics, or speculative feature.

   impact:otherThis issue has meaningful maintainer-visible impact outside the owned taxonomy.This issue has meaningful maintainer-visible impact outside the owned taxonomy.

   issue-rating: 🦞 diamond lobsterVery strong issue quality with high-confidence source-level or clear reproduction.Very strong issue quality with high-confidence source-level or clear reproduction.

   clawsweeper:source-reproClawSweeper found a high-confidence source-level issue reproduction.ClawSweeper found a high-confidence source-level issue reproduction.

   on Jun 17, 2026
3. TimPietruskyRunPod commented on Jun 17, 2026

   

   TimPietruskyRunPod

   on Jun 17, 2026

   Contributor

   More actions

   Codex review: keeping this open for maintainer follow-up; there is still a little grit to resolve. Reviewed June 17, 2026, 9:39 AM ET / 13:39 UTC.

   Summary
   Current main and release 5.8.6 still do not implement BUCKET_PREFIX; the issue should stay open because #191 is the open implementation candidate called out by repository discussion.

   Reproducibility: yes. Source inspection shows current main has no BUCKET_PREFIX lookup and the S3 path calls rp_upload.upload_image(job_id, temp_file_path), so the requested prefix cannot affect output keys today.

   Next step
   No separate repair lane is needed while the linked implementation pull request remains open as the candidate fix.

   Review details

   Best possible solution:

   Review and land #191, or replace it with an equivalent narrow change that preserves default S3 upload behavior when BUCKET_PREFIX is unset.

   Do we have a high-confidence way to reproduce the issue?

   Yes. Source inspection shows current main has no BUCKET_PREFIX lookup and the S3 path calls rp_upload.upload_image(job_id, temp_file_path), so the requested prefix cannot affect output keys today.

   Is this the best way to solve the issue?

   Yes. Optional prefix handling with unchanged default behavior is the narrowest solution, and the open linked pull request is already the current candidate implementation.

   AGENTS.md: found and applied where relevant.

   Remaining risk / open question:

   • Users may assume BUCKET_PREFIX is available because the candidate implementation exists, but current main and release 5.8.6 still ignore it.

   Codex review notes: model internal, reasoning high; reviewed against 066a11c49cfe.

   Label changes

   Label changes:

   • add P3: This is an optional S3 upload configuration improvement with limited blast radius.
   • add impact:other: The issue affects how generated output objects are organized in external S3 storage, which is maintainer-visible but outside the specific impact taxonomy.
   • add issue-rating: 🦞 diamond lobster: Current issue advisory state selects this label.
   • add clawsweeper:source-repro: Current issue advisory state selects this label.

   Label justifications:

   • P3: This is an optional S3 upload configuration improvement with limited blast radius.
   • impact:other: The issue affects how generated output objects are organized in external S3 storage, which is maintainer-visible but outside the specific impact taxonomy.

   Evidence reviewed

   What I checked:

   • Repository policy read: AGENTS.md was read fully; its Docker rebuild caveat matters because handler changes need an image rebuild for runtime validation. (AGENTS.md:7, 066a11c49cfe)
   • Current upload path lacks prefix support: The S3 branch only checks BUCKET_ENDPOINT_URL and calls rp_upload.upload_image(job_id, temp_file_path); no BUCKET_PREFIX value is read. (handler.py:780, 066a11c49cfe)
   • Repository-wide search found no implementation: BUCKET_PREFIX does not appear in current source, tests, docs, README, or changelog. (066a11c49cfe)
   • Docs describe job-id based upload behavior: Configuration docs say the RunPod upload helper creates a unique bucket path based on job_id, with no prefix configuration documented. (docs/configuration.md:43, 066a11c49cfe)
   • Existing tests assert current helper call: The S3 test expects upload_image(job_id, path), which matches current behavior and has no prefix assertion. (tests/test_handler.py:173, 066a11c49cfe)
   • Related implementation candidate: Provided GitHub context includes a repository member comment stating that feat: add BUCKET_PREFIX support for S3 uploads #191 implements this behavior and should be ready after a changeset entry.

   Likely related people:

   • jpswelch: Opened the linked implementation pull request that adds BUCKET_PREFIX handling across handler.py, docs, and tests. (role: candidate implementation author; confidence: medium; commits: b5e26d8a4f88; files: handler.py, tests/test_handler.py, docs/configuration.md)
   • TimPietruskyRunPod: Repository member comment says they reviewed the linked implementation and identified the remaining changeset/release-validation path. (role: reviewer and adjacent owner; confidence: medium; commits: 5213d7890e87; files: .github/workflows/dev.yml)

   How this review workflow works

   • ClawSweeper keeps one durable marker-backed review comment per issue or PR.
   • Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
   • A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
   • PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
   • Maintainers can also comment @clawsweeper review to request a fresh review only.
   • Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
   • Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
   • Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.