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.

Sign up for GitHub

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

Jump to bottom

feat(s3): Support S3 Region #12063

Closed
wants to merge 3 commits into from
Closed

feat(s3): Support S3 Region #12063

wants to merge 3 commits into from

Conversation

majetideepak
Copy link
Collaborator

@majetideepak majetideepak commented Jan 11, 2025 •
edited
Loading

We will follow a subset of the Hadoop aws library rules to be consistent with other applications. These rules seem reasonable.
https://hadoop.apache.org/docs/current/hadoop-aws/tools/hadoop-aws/connecting.html#s3_endpoint_region_details

  1. Configs hive.s3.endpoint and hive.s3.endpoint.region are used to set values for S3 endpoint and region respectively.
  2. If hive.s3.endpoint.region is configured with valid AWS region value, S3FS will use this value.
  3. If hive.s3.endpoint.region is not set and hive.s3.endpoint is set with valid endpoint value, S3FS will attempt to parse the region from the endpoint and configure the client to use the region value.
  4. If both hive.s3.endpoint and hive.s3.endpoint.region are not set, S3FS will use us-east-2 as default region and enable cross-region access. In this case, S3FS does not attempt to override the endpoint while configuring the S3 client.

Fixes: #11627

@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Jan 11, 2025
@majetideepak majetideepak requested a review from JkSelf January 11, 2025 01:46
@netlify Netlify
Copy link

netlify bot commented Jan 11, 2025 •
edited
Loading

✅ Deploy Preview for meta-velox canceled.

Name Link
🔨 Latest commit 4399fa1
🔍 Latest deploy log https://app.netlify.com/sites/meta-velox/deploys/67851433a457b9000821e76d

JkSelf
JkSelf reviewed Jan 13, 2025
View reviewed changes
Copy link
Collaborator

@JkSelf JkSelf left a comment

Choose a reason for hiding this comment

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

LGTM. Just two minor nits.

velox/connectors/hive/storage_adapters/s3fs/S3Util.h Show resolved Hide resolved
velox/connectors/hive/storage_adapters/s3fs/S3Util.cpp Outdated Show resolved Hide resolved
@majetideepak
Copy link
Collaborator Author

@JkSelf thanks. I addressed your comments.

@majetideepak majetideepak requested a review from pedroerp January 13, 2025 22:01
JkSelf
JkSelf approved these changes Jan 14, 2025
View reviewed changes
Copy link
Collaborator

@JkSelf JkSelf 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.

@JkSelf JkSelf added the ready-to-merge PR that have been reviewed and are ready for merging. PRs with this tag notify the Velox Meta oncall label Jan 14, 2025
@facebook-github-bot
Copy link
Contributor

@pedroerp has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

@facebook-github-bot
Copy link
Contributor

@pedroerp merged this pull request in 93523f8.

@majetideepak majetideepak deleted the s3-region branch January 18, 2025 13:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@JkSelf JkSelf JkSelf approved these changes

@pedroerp pedroerp Awaiting requested review from pedroerp

Assignees
No one assigned
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. Merged ready-to-merge PR that have been reviewed and are ready for merging. PRs with this tag notify the Velox Meta oncall
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Add support for S3 Region
3 participants
@majetideepak @facebook-github-bot @JkSelf