-
Notifications
You must be signed in to change notification settings - Fork 799
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
[TM DownloadFile Pause and Resume] Part 2: Implement pause for downloadFile operation #3094
[TM DownloadFile Pause and Resume] Part 2: Implement pause for downloadFile operation #3094
Conversation
...ustom/s3-transfer-manager/src/main/java/software/amazon/awssdk/transfer/s3/FileDownload.java
Outdated
Show resolved
Hide resolved
public final class PersistableFileDownload implements PersistableTransfer, | ||
ToCopyableBuilder<PersistableFileDownload.Builder, PersistableFileDownload> { |
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.
How can users serialize this to, e.g., a disk or database?
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.
I think they can use any serialization/deserialization library to do that.
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.
It seems like we should consider the option of vending an opaque (Base32 or Base64) string/token that can be used to resume a download.
.../s3-transfer-manager/src/main/java/software/amazon/awssdk/transfer/s3/S3TransferManager.java
Outdated
Show resolved
Hide resolved
...nager/src/main/java/software/amazon/awssdk/transfer/s3/exception/TransferPauseException.java
Outdated
Show resolved
Hide resolved
...r-manager/src/main/java/software/amazon/awssdk/transfer/s3/internal/DefaultFileDownload.java
Outdated
Show resolved
Hide resolved
.../src/main/java/software/amazon/awssdk/transfer/s3/internal/progress/DownloadFileMonitor.java
Outdated
Show resolved
Hide resolved
.../src/main/java/software/amazon/awssdk/transfer/s3/internal/progress/DownloadFileMonitor.java
Outdated
Show resolved
Hide resolved
.../main/java/software/amazon/awssdk/transfer/s3/internal/progress/TransferListenerContext.java
Outdated
Show resolved
Hide resolved
.../main/java/software/amazon/awssdk/transfer/s3/internal/progress/TransferProgressUpdater.java
Outdated
Show resolved
Hide resolved
@@ -204,6 +205,8 @@ private Context() { | |||
*/ | |||
TransferObjectRequest request(); | |||
|
|||
SdkResponse initialResponse(); |
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.
Javadoc!
Also, can this be a S3Response
?
private final long bytesTransferred; | ||
private final Instant lastModified; | ||
|
||
private PersistableFileDownload(DefaultBuilder builder) { |
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.
Wondering if we should include a (perhaps optional) checksum of the bytes written so far, which a resume would validate before appending to an existing file. This would help ensure we don't clobber a file if it was modified by another source.
...r-manager/src/main/java/software/amazon/awssdk/transfer/s3/internal/DefaultFileDownload.java
Outdated
Show resolved
Hide resolved
...r-manager/src/main/java/software/amazon/awssdk/transfer/s3/internal/DefaultFileDownload.java
Show resolved
Hide resolved
3a576e2
to
9a4a68e
Compare
SonarCloud Quality Gate failed. |
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.
* [TM DownloadFile Pause and Resume] Part 1: Add configuration to enable overwriting existing files (#3125) * Expose an option to overwrite an existing file in FileAsyncResponseTransformer * Add changelog entries and make TM use CREATE_OR_REPLACE_EXISTING write option by default * Address feedback * Update and address feedback * [TM DownloadFile Pause and Resume] Part 2: Implement pause for downloadFile operation (#3094) * Part 1: Implement pause for downloadFile operation * Address feedback * Refactor the logic * Address feedback * Fix merging issue * [TM DownloadFile Pause and Resume] Part 3: Implement resumeDownloadFile (#3157) * Implement resumeDownloadFile * Move test code around * Address feedback * Fix flaky test * fix flaky integ test * add changelog entry * Troubleshooting flaky test * Add file length check when checking if file has modified or not * Address feedback
Motivation and Context
This is the part 2 of the implementation to support pause and resume for downloadFile operation
This PR will be merged to a feature branch once approved.
Modifications
This is still in draft.Add support for pause fordownloadFile
operationTesting
Added unit tests and integ tests
Screenshots (if appropriate)
Types of changes
Checklist
mvn install
succeedsscripts/new-change
script and following the instructions. Commit the new file created by the script in.changes/next-release
with your changes.License