Skip to content

Conversation

@400Ping
Copy link
Contributor

@400Ping 400Ping commented Jan 12, 2026

Why are these changes needed?

Summary

Migrates the historyserver S3 client from AWS SDK v1 to v2, covering client initialization, error handling, pagination, and MinIO path‑style behavior, plus updates the e2e tests and module dependencies.

Changes

  • s3.go: switch to v2 config/client, paginator, and smithy error handling; keep bucket auto‑create and MinIO path‑style.
  • config.go: remove v1 aws.Bool usage; use bool + parsing helpers.
  • collector_test.go: update e2e S3 client/paginator/types to v2.

Related issue number

Closes #4280

Checks

  • I've made sure the tests are passing.
  • Testing Strategy
    • Unit tests
    • Manual tests
    • This PR is not tested :(

Signed-off-by: 400Ping <fourhundredping@gmail.com>
@400Ping 400Ping marked this pull request as draft January 12, 2026 16:35
Signed-off-by: Ping <fourhundredping@gmail.com>
Signed-off-by: 400Ping <fourhundredping@gmail.com>
Signed-off-by: 400Ping <fourhundredping@gmail.com>
Signed-off-by: 400Ping <fourhundredping@gmail.com>
@400Ping 400Ping force-pushed the feature/migrate-to-new-AWS-S3-SDK branch from 8cf62f2 to b80d431 Compare January 15, 2026 14:43
Signed-off-by: 400Ping <jiekai.chang326@gmail.com>
@400Ping 400Ping force-pushed the feature/migrate-to-new-AWS-S3-SDK branch from cbae15b to 80be943 Compare January 15, 2026 14:57
Signed-off-by: 400Ping <jiekai.chang326@gmail.com>
Signed-off-by: 400Ping <jiekai.chang326@gmail.com>
Signed-off-by: 400Ping <jiekai.chang326@gmail.com>
@400Ping 400Ping force-pushed the feature/migrate-to-new-AWS-S3-SDK branch from ceaefd0 to 9e3c1e4 Compare January 16, 2026 16:51
Signed-off-by: Jie-Kai Chang <jiekai.chang326@gmail.com>
Signed-off-by: 400Ping <jiekai.chang326@gmail.com>
Signed-off-by: 400Ping <jiekai.chang326@gmail.com>
@400Ping 400Ping marked this pull request as ready for review January 17, 2026 10:16
Copy link
Member

@Future-Outlier Future-Outlier left a comment

Choose a reason for hiding this comment

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

Hi, @400Ping
do you mind solve the merge conflict? thank you!
also cc @fsNick @win5923 @JiangJiaWei1103 for review

Signed-off-by: Jie-Kai Chang <jiekai.chang326@gmail.com>
cursor[bot]

This comment was marked as outdated.

Signed-off-by: 400Ping <jiekai.chang326@gmail.com>
Comment on lines 378 to 380
PartitionID: "aws",
URL: endpoint,
SigningRegion: "us-east-1",
Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks for contributing! Would you mind defining the partition and the region as constants so they can be reused?

For example:

config.WithRegion("us-east-1"),

here can be modified to:

config.WithRegion(s3Region),

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done in 54764d6

Signed-off-by: 400Ping <jiekai.chang326@gmail.com>
Signed-off-by: 400Ping <jiekai.chang326@gmail.com>
awsconfig.WithHTTPClient(httpClient),
}
if endpoint != "" {
resolver := aws.EndpointResolverWithOptionsFunc(
Copy link
Member

Choose a reason for hiding this comment

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

image

aws.EndpointResolverWithOptionsFunc is deprecated.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for the catch, updated in e0962df

Signed-off-by: 400Ping <jiekai.chang326@gmail.com>
Signed-off-by: Jie-Kai Chang <jiekai.chang326@gmail.com>
@400Ping 400Ping requested a review from win5923 January 21, 2026 10:41
Copy link

@cursor cursor bot left a comment

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes and found 2 potential issues.

Signed-off-by: 400Ping <jiekai.chang326@gmail.com>
Signed-off-by: 400Ping <jiekai.chang326@gmail.com>
Signed-off-by: 400Ping <jiekai.chang326@gmail.com>
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.

[history server][collector] Migrate to new AWS S3 SDK version

4 participants