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

AWS: Add support for enabling access to S3 Requester Pays bucket #11915

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

blitzmohit
Copy link

@blitzmohit blitzmohit commented Jan 5, 2025

Took a stab at resolving #11912

  • Adds a new option s3.requester-pays-enabled which defaults to false.
  • Modify the default aws client factories to add the requester pays header via override configuration if the option is enabled

Note will also need to document the option, hoping to get some feedback on the PR before that esp. since this is my first time working with iceberg.

@github-actions github-actions bot added the AWS label Jan 5, 2025
Copy link
Contributor

@steveloughran steveloughran left a comment

Choose a reason for hiding this comment

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

This isn't the way to do it. Just call withRequesterPays(true) when creating the aws s3 client and let it do all the work

@blitzmohit
Copy link
Author

This isn't the way to do it. Just call withRequesterPays(true) when creating the aws s3 client and let it do all the work

Hi,

Thank you for reviewing the PR. I was unable to locate withRequesterPays(true) or any other built-in option to set the RequesterPays flag globally on the S3 client builder.

I do observe this method on individual requests such as UploadPartRequest and ListObjectsV2Request, etc.

However, in my opinion, this approach would be more challenging to maintain as it would require setting the option correctly before each request, including those that may be added in the future.

@blitzmohit
Copy link
Author

Hi @steveloughran

I just wanted to follow up on my previous comment and would love to hear any additional thoughts or suggestions you might have.

I looked into the Hadoop S3A implementation, and it seems to use a similar approach by setting the requester-pays header, which I believe you had also reviewed.

Ref:

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