Skip to content

Download to temp file and rename atomically in download_file#792

Merged
bhearsum merged 2 commits intomozilla-releng:mainfrom
Eijebong:i-wasnt-tall-enough
Apr 27, 2026
Merged

Download to temp file and rename atomically in download_file#792
bhearsum merged 2 commits intomozilla-releng:mainfrom
Eijebong:i-wasnt-tall-enough

Conversation

@Eijebong
Copy link
Copy Markdown
Contributor

Instead of downloading to the final file, download to a temporary file then move the file into place. This prevents two different coroutines rom writing chunks one after another into the same file. This feels a bit like a bandaid for something that shouldn't happen in the first place (we shouldn't be trying to download the same file twice) but it's a safe patch for a problem that is very real anyway as a failure would leave a corrupted file on the system right now and any retry would just see that and fail later on.

I intend on fixing the fact that we are downloading the same file multiple times in a follow up by caching the coroutine instead but this is a worthwhile fix anyway

Instead of downloading to the final file, download to a temporary file
then move the file into place. This prevents two different coroutines
from writing chunks one after another into the same file. This feels a
bit like a bandaid for something that shouldn't happen in the first
place (we shouldn't be trying to download the same file twice) but it's
a safe patch for a problem that is very real anyway as a failure would
leave a corrupted file on the system right now and any retry would just
see that and fail later on.

I intend on fixing the fact that we are downloading the same file
multiple times in a follow up by caching the coroutine instead but this
is a worthwhile fix anyway
E203 (whitespace before ':') conflicts with black's slice formatting.
The black documentation mentions this at
https://black.readthedocs.io/en/stable/guides/using_black_with_other_tools.html#e203
and is saying we should disable that rule in flake8.

I think we should switch to ruff instead and get rid of those 79 tools
we have that do the same thing slightly differently but that's a problem
for another day
@Eijebong Eijebong requested a review from a team as a code owner April 27, 2026 15:48
Copy link
Copy Markdown
Contributor

@bhearsum bhearsum left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wfm as a short term fix

@bhearsum bhearsum merged commit 71a7fee into mozilla-releng:main Apr 27, 2026
12 checks passed
bhearsum added a commit to mozilla-releng/scriptworker-scripts that referenced this pull request Apr 27, 2026
This picks up mozilla-releng/scriptworker#792, which fixes a regression from 63.0.0
bhearsum added a commit to mozilla-platform-ops/ronin_puppet that referenced this pull request Apr 28, 2026
This will pick up a new scriptworker version to pick up mozilla-releng/scriptworker#792, which fixes some CoT verification errors.
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