Skip to content

Add overwrite parameter to IMAP hook download_mail_attachments#66850

Open
kyupark wants to merge 2 commits into
apache:mainfrom
kyupark:fix-imap-overwrite-65870
Open

Add overwrite parameter to IMAP hook download_mail_attachments#66850
kyupark wants to merge 2 commits into
apache:mainfrom
kyupark:fix-imap-overwrite-65870

Conversation

@kyupark
Copy link
Copy Markdown
Contributor

@kyupark kyupark commented May 13, 2026

Add an overwrite parameter to the IMAP hook's download_mail_attachments() method.

When overwrite=True (default), duplicate filenames are overwritten (current behavior).
When overwrite=False, unique filenames are generated by appending a suffix (e.g. report_1.csv, report_2.csv).

The non-overwrite path uses exclusive create mode ("xb") to avoid TOCTOU races, and checks both in-memory state and the filesystem to handle files left over from previous runs.

closes: #65870


Was generative AI tooling used to co-author this PR?
  • Yes — Claude Code (Opus 4.7)

Generated-by: Claude Code (Opus 4.7) following the guidelines

Comment thread providers/imap/tests/unit/imap/hooks/test_imap.py
@potiuk
Copy link
Copy Markdown
Member

potiuk commented May 18, 2026

@kyupark — There is 1 unresolved review thread on this PR from phanikumv, and you have engaged with it (post-review commits and/or in-thread replies). Could you confirm whether you believe the feedback is fully addressed and the PR is ready for maintainer review confirmation?

If yes, please mark the thread as resolved and ping the reviewer (phanikumv) for a final look. They will either label the PR ready for maintainer review or follow up with additional feedback.

If you are still working on the thread, please reply with what is outstanding so the threads stay unresolved on purpose.


Note: This comment was drafted by an AI-assisted triage tool and may contain mistakes. Once you have addressed the points above, an Apache Airflow maintainer — a real person — will take the next look at your PR. We use this two-stage triage process so that our maintainers' limited time is spent where it matters most: the conversation with you.


Drafted-by: Claude Code (Opus 4.7); reviewed by @potiuk before posting

@kyupark
Copy link
Copy Markdown
Contributor Author

kyupark commented May 19, 2026

@phanikumv I added the focused FileExistsError retry test you suggested: the first open(..., "xb") for report.csv raises FileExistsError, then the retry succeeds with report_1.csv.

Local checks passed and CI is green. I’ve marked the thread resolved. Could you take a final look when you have a chance?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

IMAP hook silently overwrites attachments with identical filenames

3 participants