-
Notifications
You must be signed in to change notification settings - Fork 858
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add download implementation for API requests #1231
Add download implementation for API requests #1231
Conversation
* @param <ResponseT> Pojo Response type. | ||
* @return AsyncResponseTransformer instance. | ||
*/ | ||
static <ResponseT> AsyncResponseTransformer<ResponseT, ResponseT> toFile(Path path, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
note: I have another PR for this in #1222 that will be merged to master
. I will update the s3-transfermanager
branch once it goes through. Just didn't want it to hold this PR up.
6cc2e18
to
bd8f532
Compare
Should we look into doing experimental releases to Maven? |
...c/test/java/software/amazon/awssdk/core/internal/async/FileAsyncResponseTransformerTest.java
Outdated
Show resolved
Hide resolved
...ermanager/src/it/java/software/amazon/awssdk/custom/s3/transfer/DownloadIntegrationTest.java
Show resolved
Hide resolved
.../src/main/java/software/amazon/awssdk/custom/s3/transfer/MultipartDownloadConfiguration.java
Outdated
Show resolved
Hide resolved
...er/src/main/java/software/amazon/awssdk/custom/s3/transfer/MultipartUploadConfiguration.java
Outdated
Show resolved
Hide resolved
...er/src/main/java/software/amazon/awssdk/custom/s3/transfer/MultipartUploadConfiguration.java
Show resolved
Hide resolved
...ermanager/src/main/java/software/amazon/awssdk/custom/s3/transfer/TransferObjectRequest.java
Show resolved
Hide resolved
...r/src/main/java/software/amazon/awssdk/custom/s3/transfer/TransferOverrideConfiguration.java
Show resolved
Hide resolved
...c/main/java/software/amazon/awssdk/custom/s3/transfer/internal/TransferManagerUtilities.java
Show resolved
Hide resolved
...anager/src/main/java/software/amazon/awssdk/custom/s3/transfer/internal/DownloadManager.java
Show resolved
Hide resolved
...src/main/java/software/amazon/awssdk/custom/s3/transfer/internal/RequestConversionUtils.java
Show resolved
Hide resolved
private void validateConfig() { | ||
Validate.isPositive(multipartUploadThreshold, "multipartUploadThreshold"); | ||
Validate.isPositive(minUploadPartSize, "minUploadPartSize"); | ||
Validate.isTrue(minUploadPartSize <= multipartUploadThreshold, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why minUploadPartSize
should be less than multipartUploadThreshold
? It makes sense but should not be a requirement. Customer might have large objects like 1 GB and they want part size to be 50 MB or so. In that we should do 50MB parts instead of forcing them to 16MB parts.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Makes sense. I'll change to 5MiB, since that's the absolute min set by S3.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Had one comment. Other than that, lgtm.
1b951c5
to
f94555a
Compare
…30a0c1726 Pull request: release <- staging/3ae227e0-77a6-4cca-ac60-79a30a0c1726
Description
Basic implementation for
download
.Motivation and Context
Testing
New unit and integration tests.
Screenshots (if appropriate)
Types of changes
Checklist
mvn install
succeedsLicense