Never assemble a download at its destination path - #341
Merged
Conversation
nuwang
had a problem deploying
to
cloud-integration
July 31, 2026 07:33 — with
GitHub Actions
Failure
nuwang
had a problem deploying
to
cloud-integration
July 31, 2026 07:33 — with
GitHub Actions
Failure
download_to_file built the object at the destination: the generic ranged driver (GCP and OpenStack Swift) created the file up front and reopened it by path for every range, and the Azure downloader wrote in place. Callers commonly download every copy of an object to one well-known path - a download cache keyed by object, say - so a second download of the same object could truncate the first's file, and renaming that path into place mid-transfer left the other download reopening a path that no longer existed, failing with FileNotFoundError. A failed transfer also deleted whatever was already at the destination. Assemble into a private sibling file and rename it into place once complete, so the destination only ever holds a whole object and a failed transfer leaves an existing one untouched. Providers now fill a caller- owned path via _download_to_path and inherit that guarantee. Ranges are written through the single handle the driver opened, so a range can never land in a file that has since been replaced.
nuwang
force-pushed
the
atomic-ranged-download
branch
from
August 1, 2026 18:38
15c0bfd to
836da4e
Compare
nuwang
temporarily deployed
to
cloud-integration
August 1, 2026 18:39 — with
GitHub Actions
Inactive
nuwang
temporarily deployed
to
cloud-integration
August 1, 2026 18:39 — with
GitHub Actions
Inactive
nuwang
temporarily deployed
to
cloud-integration
August 1, 2026 18:39 — with
GitHub Actions
Inactive
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.
The bug
A Galaxy deployment on OpenStack Swift hit this downloading a 4.3 GB dataset:
download_to_filebuilt the object at the destination path: the generic ranged driver created the file up front and reopened it by path for every range, and the Azure downloader wrote in place. Callers commonly download every copy of an object to one well-known path — Galaxy's dataset cache uses a fixed<dataset>.tmpper dataset — so a second download of the same object shares that path. When one download finished and renamed the path into place, the other's next range reopened a path that no longer existed and blew up; before that, itsopen(path, 'wb')had already truncated the first download's in-progress file. A failed transfer alsoos.removed the destination, destroying whatever was previously downloaded there.Only GCP and OpenStack Swift were exposed to the crash: AWS overrides the download with boto3's TransferManager, which already assembles in its own temp file — which is why this never showed up in the mock/MinIO tests (they run the AWS provider) and only surfaced against Swift. Azure was exposed to the partial-file and destroyed-destination variants.
The fix
download_to_fileassembles into a private sibling file andos.replaces it into place once complete, so the destination only ever holds a whole object: an existing file is replaced atomically, a failed transfer leaves it untouched, and concurrent downloads to one path are safe with the last to complete winning. Providers now implement_download_to_path(filling a caller-owned path) instead of overridingdownload_to_file, so boto3, Azure and the generic driver all inherit the guarantee.The ranged driver additionally writes through the single handle it opened rather than reopening the path per range — a range can no longer land in a file that has since been replaced, and 80+ opens become one. Writes are serialized with a lock, which costs little next to the network fetches and keeps the memory bound at ~
concurrency × part_size.The interface docstring now states the guarantee explicitly.
Tests
Three new cases in
tests/test_download_driver.py, all red before the fix:Verification
tox -e py3.13-mockgreen;tox -e mypyandtox -e lintclean