Skip to content

add completed to ResumableFileDownload ser/des #5254

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

Merged
merged 3 commits into from
May 30, 2024

Conversation

L-Applin
Copy link
Contributor

Add completedParts field to ResumableFileDownload with required serialization and deserialization.

Motivation and Context

Needed to Multipart Pause/Resume logic. The ResumableFileDownload can be serialized to json and deserialized back into memory to resume a download for it.

Modifications

  • Add new field to ResumableFileDownload + Builder
  • Add a new TransferManagerJsonUnmarshaller for list of Integer
  • Update marshalling/unmarshalling to include completedParts list

Testing

Added unit tests

@L-Applin L-Applin requested a review from a team as a code owner May 29, 2024 17:51
@L-Applin L-Applin merged commit ee6ebff into feature/master/s3mpu May 30, 2024
14 of 16 checks passed
* @return part numbers of a multipart download that were completed saved to file.
*/
public List<Integer> completedParts() {
return Collections.unmodifiableList(completedParts);
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we wrap with umodifableList in the ctor instead of the getter? Also, we should probably create a new list. Example: https://github.com/aws/aws-sdk-java-v2/blob/master/core/sdk-core/src/main/java/software/amazon/awssdk/core/client/config/ClientOverrideConfiguration.java#L148

Copy link
Contributor Author

Choose a reason for hiding this comment

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

will address in full resume PR

@@ -61,6 +63,7 @@ public final class ResumableFileDownload implements ResumableTransfer,
private final Instant s3ObjectLastModified;
private final Long totalSizeInBytes;
private final Instant fileLastModified;
private final List<Integer> completedParts;
Copy link
Contributor

Choose a reason for hiding this comment

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

Is the reason for List that we may download different parts in parallel in the future?

Copy link
Contributor Author

@L-Applin L-Applin May 30, 2024

Choose a reason for hiding this comment

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

yes, trying to prepare for when we will do parallel write to file as well

@L-Applin L-Applin deleted the olapplin/resume-file-dl-serdes branch January 16, 2025 14:33
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.

3 participants