Skip to content

Use CRT response file in low level CRT S3AsyncClient for getObject #6289

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 5 commits into from
Jul 28, 2025

Conversation

alextwoods
Copy link
Contributor

@alextwoods alextwoods commented Jul 22, 2025

Use CRT response file in low level CRT S3AsyncClient for getObject.

Motivation and Context

awslabs/aws-crt-java#825 added support for setting the response file and allowing CRT to write directly to the file. That can improve memory usage by avoiding passing the chunks into java where they are then written to the file.

Note that this change does not change the Transfrer manager behavior - TransferManager will still use the Java based in memory transform.

Modifications

This follows the implementation for requestPath (the inverse feature, allowing paths to be set for putObject, see: #4379).

  • Implement getObject(*, Path) in the DefaultS3CrtAsyncClient. Sets internal execution attributes for response file.
  • S3CrtAsyncHttpClient - plumb through execution attributes

Testing

New and existing s3 tests, including TM + regression tests.

Screenshots (if appropriate)

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)

Checklist

  • I have read the CONTRIBUTING document
  • Local run of mvn install succeeds
  • My code follows the code style of this project
  • My change requires a change to the Javadoc documentation
  • I have updated the Javadoc documentation accordingly
  • I have added tests to cover my changes
  • All new and existing tests passed
  • I have added a changelog entry. Adding a new entry must be accomplished by running the scripts/new-change script and following the instructions. Commit the new file created by the script in .changes/next-release with your changes.
  • My change is to implement 1.11 parity feature and I have updated LaunchChangelog

License

  • I confirm that this pull request can be released under the Apache 2 license

@alextwoods alextwoods marked this pull request as ready for review July 22, 2025 22:03
@alextwoods alextwoods requested a review from a team as a code owner July 22, 2025 22:03
@alextwoods alextwoods changed the title Use CRT response file in getObject Use CRT response file in low level CRT S3AsyncClient for getObject Jul 22, 2025
@zoewangg zoewangg requested a review from Copilot July 23, 2025 16:22
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR implements support for using CRT's response file feature in the low-level CRT S3AsyncClient for getObject operations. This enhancement allows the CRT library to write response data directly to a file, improving memory usage by avoiding passing data chunks through Java.

Key changes:

  • Added a new getObject(GetObjectRequest, Path) method to DefaultS3CrtAsyncClient that uses CRT's direct file writing
  • Implemented execution attribute plumbing to pass response file path and options through the request pipeline
  • Created a new response transformer that acts as a no-op when CRT handles file writing directly

Reviewed Changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
DefaultS3CrtAsyncClient.java Added getObject method with Path parameter and execution attribute mapping
S3CrtAsyncHttpClient.java Enhanced to handle response file path and options from execution attributes
CrtResponseFileResponseTransformer.java New no-op response transformer for CRT-managed file responses
S3InternalSdkHttpExecutionAttribute.java Added new execution attributes for response file path and options
S3CrtAsyncHttpClientTest.java Added test coverage for response file functionality
CrtResponseFileResponseTransformerTest.java Comprehensive test suite for the new response transformer
CodingConventionWithSuppressionTest.java Updated suppressions to include new transformer class
feature-AWSS3-233f74c.json Changelog entry for the new feature

Copy link

Quality Gate Failed Quality Gate failed

Failed conditions
63.8% Coverage on New Code (required ≥ 80%)
B Reliability Rating on New Code (required ≥ A)

See analysis details on SonarQube Cloud

Catch issues before they fail your Quality Gate with our IDE extension SonarQube for IDE

@alextwoods alextwoods added this pull request to the merge queue Jul 28, 2025
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to no response for status checks Jul 28, 2025
@alextwoods alextwoods added this pull request to the merge queue Jul 28, 2025
Merged via the queue into master with commit 2120ac6 Jul 28, 2025
41 of 42 checks passed
Copy link

This pull request has been closed and the conversation has been locked. Comments on closed PRs are hard for our team to see. If you need more assistance, please open a new issue that references this one.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jul 28, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants