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

Dont use DNS style addressing for GetBucketLocation #380

Merged
merged 2 commits into from
Nov 18, 2014

Conversation

jamesls
Copy link
Member

@jamesls jamesls commented Nov 17, 2014

This fixes an issue when you try to get bucket location
for a bucket in eu-central-1. You now do not need a region
specified.

For example, you previously needed:

$ aws s3api get-bucket-location --bucket jamesls-eu-central-1 --region eu-central-1
{
    "LocationConstraint": "eu-central-1"
}

which defeated the purpose of get-bucket-location. Now the region doesn't matter:

$ aws s3api get-bucket-location --bucket jamesls-eu-central-1 --region us-east-1
{
    "LocationConstraint": "eu-central-1"
}
$ aws s3api get-bucket-location --bucket jamesls-eu-central-1 --region us-west-2
{
    "LocationConstraint": "eu-central-1"
}
$ aws s3api get-bucket-location --bucket jamesls-eu-central-1 --region ap-northeast-1
{
    "LocationConstraint": "eu-central-1"
}

cc @kyleknap @danielgtaylor

This bypasses an issue when you try to get bucket location
for a bucket in eu-central-1.  You now do not need a region
specified.
# Also keep in mind that while this test is useful, it doesn't test
# what happens once DNS propogates which is arguably more interesting,
# as DNS will point us to the eu-central-1 endpoint.
response = operation.call(self.endpoint, Bucket=self.bucket_name)
Copy link
Contributor

Choose a reason for hiding this comment

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

Is the endpoint region being used is "eu-central-1" still? I would like to see using different regions for endpoints.

Copy link
Member Author

Choose a reason for hiding this comment

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

It's not obvious from this test, but self.endpoint is actually the endpoint for us-east-1, defined in the base classes's setUp.

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh my bad. I saw endpoint in the setUp() and assumed it was self.endpoint

@kyleknap
Copy link
Contributor

LGTM. Just had some questions about the tests.

@jamesls
Copy link
Member Author

jamesls commented Nov 17, 2014

@kyleknap fixed up the tests to use clients in the text fixtures. Also just went ahead and created a new us-east-1 endpoint to make it more clear that we're testing us-east-1.

@kyleknap
Copy link
Contributor

Thanks. LGTM. 🚢

@jamesls jamesls merged commit e630f2d into boto:develop Nov 18, 2014
jamesls added a commit that referenced this pull request Nov 18, 2014
* no-dns-bucket-location:
  Fix up test to use client objects in test fixtures
  Dont use DNS style addressing for GetBucketLocation

Conflicts:
	tests/unit/test_handlers.py

Closes #380
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.

2 participants