Skip to content

Conversation

@hsiang-c
Copy link
Contributor

@hsiang-c hsiang-c commented Oct 7, 2024

Context

  • In the iceberg-aws module, the community has added the following S3 configurations over time:
S3 configuration Pull Request
Signer configuration #7562
UserAgent configuration #9963
Retry configuration #11052
  • As PR AWS: Make sure Signer + User Agent config are both applied #10198 suggested, calling builder.overrideConfiguration() will return a new ClientOverrideConfiguration instance and ignore existing overridden configurations.
  • However, the previous PR didn't take care of the Retry configuration b/c it was introduced only a few days ago.

Proposed changes

  • Make sure we can apply the following configurations without losing conf values.
s3Properties.applySignerConfiguration(builder);
s3Properties.applyUserAgentConfigurations(builder);
s3Properties.applyRetryConfigurations(builder);

@github-actions github-actions bot added the AWS label Oct 7, 2024
@hsiang-c
Copy link
Contributor Author

hsiang-c commented Oct 7, 2024

@ookumuso @nastra Please review the fix when you have time, thanks.

Copy link
Contributor

@nastra nastra left a comment

Choose a reason for hiding this comment

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

LGTM, thanks for catching and fixing this @hsiang-c

@nastra nastra merged commit 745e819 into apache:main Oct 7, 2024
@hsiang-c
Copy link
Contributor Author

hsiang-c commented Oct 7, 2024

Thank you for your review @nastra

@hsiang-c hsiang-c deleted the keep_overridden_conf branch October 7, 2024 12:28
zachdisc pushed a commit to zachdisc/iceberg that referenced this pull request Dec 23, 2024
parthchandra pushed a commit to parthchandra/iceberg that referenced this pull request Oct 22, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants