Skip to content
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

feat: support S3 Express One Zone #1033

Merged
merged 85 commits into from
Feb 28, 2024
Merged

feat: support S3 Express One Zone #1033

merged 85 commits into from
Feb 28, 2024

Conversation

lauzadis
Copy link
Contributor

@lauzadis lauzadis commented Feb 8, 2024

Adds some features in order to support the SDK making S3 Express requests

Issue #

Description of changes

  • Refactor checksums to use an execution context key HttpOperationContext.ChecksumAlgorithm. The value is the name of the checksum algorithm to be used. If it's set when flexible checksums runs, it will be used, otherwise the Content-MD5 header will be computed and added as before. This greatly simplifies the logic required in these interceptors.
  • Allow setting omitSessionToken signing configuration via execution context, using the AwsSigningAttributes. OmitSessionToken key.
  • Add a generic, thread-safe LruCache which holds entries up to a configurable capacity and then evicts the least-recently used entries as more are added.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

*/
@InternalApi
public class Md5ChecksumInterceptor<I>(
private val block: ((input: I) -> Boolean)? = null,
) : HttpInterceptor {
private var cachedChecksum: String? = null
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't quite understand what would break. Md5ChecksumInterceptor and FlexibleChecksumsRequestIntereceptor are final so it seems like users can't create their own custom implementations.

public class LruCache<K, V>(
public val capacity: Int,
) {
private val mu = Mutex() // protects map
Copy link
Contributor

Choose a reason for hiding this comment

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

thought: What if we just made this cache explicitly not thread safe and make synchronization a higher level concern? That would make things like entries and other fields thread safe if needed and allow locking to be at whatever granularity makes sense.

Just a thought.

Copy link

sonarcloud bot commented Feb 28, 2024

Quality Gate Passed Quality Gate passed

Issues
2 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud

@lauzadis lauzadis added the acknowledge-api-break Acknowledge that a change is API breaking and may be backwards-incompatible. Review carefully! label Feb 28, 2024
@aws-sdk-kotlin-ci aws-sdk-kotlin-ci merged commit 7471609 into main Feb 28, 2024
13 of 16 checks passed
@aws-sdk-kotlin-ci aws-sdk-kotlin-ci deleted the feat-s3-express branch February 28, 2024 20:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
acknowledge-api-break Acknowledge that a change is API breaking and may be backwards-incompatible. Review carefully!
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants