Repository navigation
feat(get): stream to an S3 destination (0.21.0) - #178
Merged
Merged
Conversation
`dt get -o s3://bucket/prefix/` streams DVC-tracked data from the source repository's remote (R2 or SSH) straight into object storage, never touching local disk. This is for handing data to a collaborator working in AWS: they run it on their own instance and the bytes go remote-to-bucket without either side provisioning scratch for a 358 GiB transfer. DVC hands us the source filesystem directly via Repo.cloud.get_remote(), so there is no `dvc get` subprocess per file and no contention on the clone's SQLite state db -- the constraint that forces the existing network path to run CSV rows serially. Two properties fall out of streaming: - Verification is free. Bytes pass through this process, so they are hashed in flight and compared against the md5 DVC recorded. A corrupt transfer is caught during the copy and the object deleted, rather than left behind carrying our metadata and passing --check for ever after. - Writes are atomic. s3fs uploads via multipart and the object does not exist until commit, so an interrupted transfer leaves nothing -- the truncated-file failure --check defends against locally cannot occur here. --resume/--check compare against the DVC md5 stored as object metadata, since re-hashing means downloading and an ETag is not an md5 once an upload is multipart. The limitation is documented: an object replaced out of band still looks verified. Destination credentials are separate from the source and passed explicitly, never via os.environ. An explicit profile outranks ambient AWS_* in botocore, which is what keeps the two from colliding; mutating the environment would also silently redirect `dt auth check`, which shells out to `aws` without --profile. Preflight resolves the identity, checks the bucket and probes write permission before any bytes move, printing the account and ARN unconditionally -- omitting --dest-profile is legal so an EC2 instance role needs no config, and saying which account is the only thing standing between that and writing to the wrong one. Guards for the two ways this goes quietly wrong: - `dt auth setup` writes repo-named R2 profiles carrying region = auto into the same ~/.aws/credentials as real AWS profiles, so one is easy to pass by accident. A destination whose region resolves to 'auto' without --dest-endpoint-url is rejected. - `-o s3://...` previously collapsed to the relative path 's3:/bucket/...' and silently created a local directory named 's3:'. URL destinations reaching the local path are now rejected outright. ssh:// destinations are deferred. Credentials would be free (ssh config is already the BYO mechanism), but there is no multipart equivalent, so an interrupted write leaves a truncated file and --resume would mean something materially weaker under the same flag name. See docs/get-s3.md. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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.
dt get -o s3://bucket/prefix/streams DVC-tracked data from the source repository's remote (R2 or SSH) straight into object storage, never touching local disk.This is for handing data to a collaborator working in AWS: they run it on their own instance, and the bytes go remote-to-bucket without either side provisioning scratch for a 358 GiB transfer.
How
DVC hands us the source filesystem directly, credentials and endpoint already resolved from the clone's
.dvc/config:Not a new pattern here —
_check_dvc_remote_impl(dt/auth/checks.py:849) already does this. It means nodvc getsubprocess per file and no contention on the clone's SQLite state db, the constraint that forces the existing network path to run CSV rows serially.Two properties fall out of streaming
Verification is free. Bytes pass through this process, so they're hashed in flight and compared against the md5 DVC recorded. A corrupt transfer is caught during the copy and the object deleted — rather than left behind carrying our metadata and passing
--checkfor ever after.Writes are atomic.
s3fsuploads via multipart; the object doesn't exist until commit. An interrupted transfer leaves nothing — the truncated-file failure--checkdefends against locally cannot occur here.Credentials
Destination credentials are separate from the source and passed explicitly, never via
os.environ. An explicit profile outranks ambientAWS_*in botocore, which is what keeps the two from colliding; mutating the environment would also silently redirectdt auth check, which shells out toawswithout--profile.Preflight resolves the identity, checks the bucket, and probes write permission before any bytes move — printing the account and ARN unconditionally:
Omitting
--dest-profileis legal so an EC2 instance role needs no config; saying which account is the only thing between that and writing to the wrong one.Guards for the two quiet failure modes
dt auth setupwrites repo-named R2 profiles carryingregion = autointo the same~/.aws/credentialsas real AWS profiles, so one is easy to pass by accident. A destination whose region resolves toautowithout--dest-endpoint-urlis rejected.-o s3://...previously collapsed to the relative paths3:/bucket/...and silently created a local directory nameds3:. URL destinations reaching the local path are now rejected outright.Review notes
decide()now accepts aPathor a destination object, so the resume/check matrix is written once and applies unchanged to S3. All 97 pre-existingtest_get.pytests pass untouched.resolve_link_typesencodes a hard-won EXDEV lesson (15dc42f) and has no fsspec expression.ssh://destinations are deferred. Credentials would be free (ssh config is already the BYO mechanism), but there's no multipart equivalent, so--resumewould mean something materially weaker under the same flag name. Reasoning indocs/get-s3.md.Testing
2069 passed, 1 skipped — 42 new in
tests/unit/test_get_s3.pyagainst a fake S3 filesystem, covering addressing, streaming, metadata round-trip, the fulldecide()matrix on S3, corrupt-source detection, and the credential guards.One assumption not covered: metadata surviving a real multipart upload is verified at the s3fs source level and by a fake, but never against a live bucket. Worth smoke-testing before trusting
--checkon a real handoff.Operational note
Aborted multipart uploads leave orphaned parts that keep accruing storage charges. The receiving bucket wants a lifecycle rule expiring incomplete multipart uploads — documented in
docs/get.md, not enforced in code.🤖 Generated with Claude Code