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

update the documentation for creating s3 bucket to address the region mismatch issue. #4337

Open
wants to merge 5 commits into
base: develop
Choose a base branch
from

Conversation

ubaskota
Copy link

@ubaskota ubaskota commented Nov 8, 2024

This PR adds documentation for an issue that results in IllegalLocationConstraintException error when a user tries to create a bucket in a region that's different from the region in their configuration. By default, s3 creates a bucket in us-east-1 if the region isn't defined in LocationConstraint, and if the region in user's configuration isn't set to us-east-1, its throws an error. This PR warns the users about this issue and provides them with options to prevent this.

Copy link
Contributor

@jonathan343 jonathan343 left a comment

Choose a reason for hiding this comment

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

I feel like this warning makes this example confusing for most users following this guide. As a user, I would just want to follow an example that works and worry less about the nuance of creating buckets.

Why don't we simplify the example function to set a default region value ofus-east-1 if not provided by the user? We would eliminate the if region is None conditional and always create the client in the same region as what we set for the LocationConstraint.

Copy link
Contributor

@nateprewitt nateprewitt left a comment

Choose a reason for hiding this comment

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

LGTM pending any further feedback from Jonathan.

.gitignore Outdated Show resolved Hide resolved
Copy link
Contributor

@jonathan343 jonathan343 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 @ubaskota!

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.

3 participants