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

validate S3 client #63

Closed
wants to merge 1 commit into from
Closed

Conversation

sullis
Copy link
Contributor

@sullis sullis commented Jul 24, 2024

No description provided.

@wakingrufus
Copy link
Owner

what is the benefit of these additional assertions / s3 calls? It seems like it just testing AWS code, not spring-funk code

@sullis
Copy link
Contributor Author

sullis commented Jul 25, 2024

what is the benefit of these additional assertions / s3 calls? It seems like it just testing AWS code, not spring-funk code

I am evaluating Amazon's CRT client:

#64

I wanted to have baseline tests for the S3AsyncClient. S3TestHelper helps validate that the CRT client behaves the same as the Netty client.

@wakingrufus
Copy link
Owner

what is the benefit of these additional assertions / s3 calls? It seems like it just testing AWS code, not spring-funk code

I am evaluating Amazon's CRT client:

#64

I wanted to have baseline tests for the S3AsyncClient. S3TestHelper helps validate that the CRT client behaves the same as the Netty client.

I see. this CRT client is new to me. I think i have an idea:
let's not repeat these assertions for every test. but let's keep them in the test with metrics test. but then we can add to them to pull the metrics for the operations out of the meter registry. that can establish a baseline for performance, then when we switch to CRT, we can compare the metrics that come out and make sure they are at least as good as they were with Netty. (obviously these will not be real performance metrics, as we will keep it fast and small, but its somthing).

OR

we can add a configuration property to select the client, and replicate the test with metrics for both variants. that way downstream consumers can select the impl that best fits their use case and we can still collect performance metrics for each.

@sullis
Copy link
Contributor Author

sullis commented Jul 27, 2024

This blog post (February 2023) compares the CRT client with the Netty client:

https://aws.amazon.com/blogs/developer/announcing-availability-of-the-aws-crt-http-client-in-the-aws-sdk-for-java-2-x/

@sullis
Copy link
Contributor Author

sullis commented Aug 8, 2024

This PR is not necessary.

I have a test suite in a different repo:
https://github.com/sullis/s3-playground/tree/main/src/test/java/io/github/sullis/s3/playground

@sullis sullis closed this Aug 8, 2024
@sullis sullis deleted the sean/validate-s3-client branch August 8, 2024 17:24
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.

2 participants