Skip to content
This repository has been archived by the owner on Dec 21, 2023. It is now read-only.

Fix S3 Region #90

Merged
merged 1 commit into from
Dec 15, 2017
Merged

Fix S3 Region #90

merged 1 commit into from
Dec 15, 2017

Conversation

ylow
Copy link
Collaborator

@ylow ylow commented Dec 14, 2017

S3 Regions are not set in the S3 client. We need to set it to support
anything outside of us-east-1.

S3 Regions are not set in the S3 client. We need to set it to support
anything outside of us-east-1
@ylow ylow requested a review from znation December 14, 2017 01:36
};

const std::map<std::string, std::string> AWS_S3_ENDPOINT_TO_REGION {
Copy link
Collaborator

Choose a reason for hiding this comment

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

It looks like there are other S3 regions. Do we want to add more here?

http://docs.aws.amazon.com/general/latest/gr/rande.html#s3_region

@ylow
Copy link
Collaborator Author

ylow commented Dec 14, 2017

Hmm... The oddity is that the AWS C++ SDK only has a smaller set of regions.

https://github.com/aws/aws-sdk-cpp/blob/3e979d7bc272f77ac179b4849f7386027201c1a2/aws-cpp-sdk-ec2/include/aws/ec2/model/Region.h

@znation
Copy link
Contributor

znation commented Dec 14, 2017

@ylow @TobyRoseman Do we have a decision here - should we take the larger list, from AWS S3 documentation, or the smaller list, from AWS C++ SDK documentation? Do we know whether the larger list includes regions that do actually exist in S3 that we're missing, and whether we need to add them in order for buckets in those regions to work?

@ylow
Copy link
Collaborator Author

ylow commented Dec 15, 2017

Lets add the regions in the AWS C++ SDK list first. We can always add more later.

@znation
Copy link
Contributor

znation commented Dec 15, 2017

Fixes #57.

@znation znation merged commit 91dcc16 into apple:master Dec 15, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants