txAWS

Merge lp:~niemeyer/txaws/s3-bucket-paths into lp:txaws

  1. s3-bucket-paths
  2. Merge into trunk
Proposed by Gustavo Niemeyer
Status: Merged
Merged at revision: 79
Proposed branch: lp:~niemeyer/txaws/s3-bucket-paths
Merge into: lp:txaws
Diff against target: 257 lines (+26/-61)
10 files modified
bin/txaws-create-bucket (+1/-1)
bin/txaws-delete-bucket (+1/-1)
bin/txaws-delete-object (+1/-1)
bin/txaws-get-bucket (+1/-1)
bin/txaws-get-object (+1/-1)
bin/txaws-head-object (+1/-1)
bin/txaws-list-buckets (+1/-1)
bin/txaws-put-object (+1/-1)
txaws/s3/client.py (+12/-35)
txaws/s3/tests/test_client.py (+6/-18)
To merge this branch: bzr merge lp:~niemeyer/txaws/s3-bucket-paths
Reviewer Review Type Date Requested Status
Jamu Kakar Approve
Review via email: mp+57429@code.launchpad.net

Description of the change

S3 used to work by providing a path to bucket names, and
then they started supporting a nice API via URLs. While
that tends to work well overall, it has an important
disavantage which is not working with alternative
implementations which provide an IP address only.

This change should remain compatible with Amazon's S3,
while offering the chance of interacting with the API
through an IP address.

To post a comment you must log in.
Revision history for this message
Jamu Kakar (jkakar) wrote :

[1]

There are many test failures:

[FAIL]
Traceback (most recent call last):
  File "/home/jkakar/src/projects/txaws/reviews/s3-bucket-paths/txaws/ec2/tests/test_client.py", line 1502, in test_response_parse_error
    self.assertEquals(str(error), "400 Bad Request")
twisted.trial.unittest.FailTest: not equal:
a = 'mismatched tag: line 1, column 7'
b = '400 Bad Request'

txaws.ec2.tests.test_client.EC2ErrorWrapperTestCase.test_response_parse_error
===============================================================================
[FAIL]
Traceback (most recent call last):
  File "/home/jkakar/src/projects/txaws/reviews/s3-bucket-paths/txaws/s3/tests/test_client.py", line 29, in test_get_host_with_bucket
    self.assertEquals(url_context.get_host(), "mystuff.s3.amazonaws.com")
twisted.trial.unittest.FailTest: not equal:
a = 's3.amazonaws.com'
b = 'mystuff.s3.amazonaws.com'

txaws.s3.tests.test_client.URLContextTestCase.test_get_host_with_bucket
===============================================================================
[FAIL]
Traceback (most recent call last):
  File "/home/jkakar/src/projects/txaws/reviews/s3-bucket-paths/txaws/s3/tests/test_client.py", line 42, in test_get_path_with_bucket_and_object
    self.assertEquals(url_context.get_host(), "mystuff.s3.amazonaws.com")
twisted.trial.unittest.FailTest: not equal:
a = 's3.amazonaws.com'
b = 'mystuff.s3.amazonaws.com'

txaws.s3.tests.test_client.URLContextTestCase.test_get_path_with_bucket_and_object
===============================================================================
[FAIL]
Traceback (most recent call last):
  File "/home/jkakar/src/projects/txaws/reviews/s3-bucket-paths/txaws/s3/tests/test_client.py", line 48, in test_get_path_with_bucket_and_object_without_slash
    self.assertEquals(url_context.get_host(), "mystuff.s3.amazonaws.com")
twisted.trial.unittest.FailTest: not equal:
a = 's3.amazonaws.com'
b = 'mystuff.s3.amazonaws.com'

txaws.s3.tests.test_client.URLContextTestCase.test_get_path_with_bucket_and_object_without_slash
===============================================================================
[FAIL]
Traceback (most recent call last):
  File "/home/jkakar/src/projects/txaws/reviews/s3-bucket-paths/txaws/s3/tests/test_client.py", line 63, in test_get_uri_with_endpoint_bucket_and_object
    "http://mydocs.localhost/notes.txt")
twisted.trial.unittest.FailTest: not equal:
a = 'http://localhost/mydocs/notes.txt'
b = 'http://mydocs.localhost/notes.txt'

txaws.s3.tests.test_client.URLContextTestCase.test_get_uri_with_endpoint_bucket_and_object

review: Needs Fixing
lp:~niemeyer/txaws/s3-bucket-paths updated
81. By Gustavo Niemeyer

Fixed tests, better docs, removed BucketURLContext.

Revision history for this message
Gustavo Niemeyer (niemeyer) wrote :

[1]

Oops, sorry about that. I've naively assumed there were no tests after not seeing
a test runner nor any test directories. I'm very happy to see this isn't the case,
though. :-)

Please note that the first failure you mention above is unrelated. It happens with
the current trunk already.

I've pushed a new version with more sensible changes. Please take another look.

Revision history for this message
Jamu Kakar (jkakar) wrote :

Thanks, I've merged the change!

review: Approve
Revision history for this message
Gustavo Niemeyer (niemeyer) wrote :

Thanks Jamu!

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'bin/txaws-create-bucket'
2--- bin/txaws-create-bucket 2010-01-13 20:04:32 +0000
3+++ bin/txaws-create-bucket 2011-04-13 14:52:38 +0000
4@@ -30,7 +30,7 @@
5 sys.exit(1)
6 creds = AWSCredentials(options.access_key, options.secret_key)
7 region = AWSServiceRegion(
8- creds=creds, region=options.region, s3_endpoint=options.url)
9+ creds=creds, region=options.region, s3_uri=options.url)
10 client = region.get_s3_client()
11
12 d = client.create_bucket(options.bucket)
13
14=== modified file 'bin/txaws-delete-bucket'
15--- bin/txaws-delete-bucket 2010-01-13 20:04:32 +0000
16+++ bin/txaws-delete-bucket 2011-04-13 14:52:38 +0000
17@@ -30,7 +30,7 @@
18 sys.exit(1)
19 creds = AWSCredentials(options.access_key, options.secret_key)
20 region = AWSServiceRegion(
21- creds=creds, region=options.region, s3_endpoint=options.url)
22+ creds=creds, region=options.region, s3_uri=options.url)
23 client = region.get_s3_client()
24
25 d = client.delete_bucket(options.bucket)
26
27=== modified file 'bin/txaws-delete-object'
28--- bin/txaws-delete-object 2010-01-13 20:04:32 +0000
29+++ bin/txaws-delete-object 2011-04-13 14:52:38 +0000
30@@ -34,7 +34,7 @@
31 sys.exit(1)
32 creds = AWSCredentials(options.access_key, options.secret_key)
33 region = AWSServiceRegion(
34- creds=creds, region=options.region, s3_endpoint=options.url)
35+ creds=creds, region=options.region, s3_uri=options.url)
36 client = region.get_s3_client()
37
38 d = client.delete_object(options.bucket, options.object_name)
39
40=== modified file 'bin/txaws-get-bucket'
41--- bin/txaws-get-bucket 2010-01-13 20:04:32 +0000
42+++ bin/txaws-get-bucket 2011-04-13 14:52:38 +0000
43@@ -34,7 +34,7 @@
44 sys.exit(1)
45 creds = AWSCredentials(options.access_key, options.secret_key)
46 region = AWSServiceRegion(
47- creds=creds, region=options.region, s3_endpoint=options.url)
48+ creds=creds, region=options.region, s3_uri=options.url)
49 client = region.get_s3_client()
50
51 d = client.get_bucket(options.bucket)
52
53=== modified file 'bin/txaws-get-object'
54--- bin/txaws-get-object 2010-01-13 20:04:32 +0000
55+++ bin/txaws-get-object 2011-04-13 14:52:38 +0000
56@@ -34,7 +34,7 @@
57 sys.exit(1)
58 creds = AWSCredentials(options.access_key, options.secret_key)
59 region = AWSServiceRegion(
60- creds=creds, region=options.region, s3_endpoint=options.url)
61+ creds=creds, region=options.region, s3_uri=options.url)
62 client = region.get_s3_client()
63
64 d = client.get_object(options.bucket, options.object_name)
65
66=== modified file 'bin/txaws-head-object'
67--- bin/txaws-head-object 2010-01-13 20:04:32 +0000
68+++ bin/txaws-head-object 2011-04-13 14:52:38 +0000
69@@ -35,7 +35,7 @@
70 sys.exit(1)
71 creds = AWSCredentials(options.access_key, options.secret_key)
72 region = AWSServiceRegion(
73- creds=creds, region=options.region, s3_endpoint=options.url)
74+ creds=creds, region=options.region, s3_uri=options.url)
75 client = region.get_s3_client()
76
77 d = client.head_object(options.bucket, options.object_name)
78
79=== modified file 'bin/txaws-list-buckets'
80--- bin/txaws-list-buckets 2010-01-13 20:04:32 +0000
81+++ bin/txaws-list-buckets 2011-04-13 14:52:38 +0000
82@@ -31,7 +31,7 @@
83 options, args = parse_options(__doc__.strip())
84 creds = AWSCredentials(options.access_key, options.secret_key)
85 region = AWSServiceRegion(
86- creds=creds, region=options.region, s3_endpoint=options.url)
87+ creds=creds, region=options.region, s3_uri=options.url)
88 client = region.get_s3_client()
89
90 d = client.list_buckets()
91
92=== modified file 'bin/txaws-put-object'
93--- bin/txaws-put-object 2010-01-13 20:04:32 +0000
94+++ bin/txaws-put-object 2011-04-13 14:52:38 +0000
95@@ -42,7 +42,7 @@
96 sys.exit(1)
97 creds = AWSCredentials(options.access_key, options.secret_key)
98 region = AWSServiceRegion(
99- creds=creds, region=options.region, s3_endpoint=options.url)
100+ creds=creds, region=options.region, s3_uri=options.url)
101 client = region.get_s3_client()
102
103 d = client.put_object(
104
105=== modified file 'txaws/s3/client.py'
106--- txaws/s3/client.py 2011-03-26 12:52:50 +0000
107+++ txaws/s3/client.py 2011-04-13 14:52:38 +0000
108@@ -33,9 +33,10 @@
109 class URLContext(object):
110 """
111 The hosts and the paths that form an S3 endpoint change depending upon the
112- context in which they are called. Sometimes the bucket name is in the host,
113- sometimes in the path. What's more, the behaviour against live AWS
114- resources doesn't seem to match the AWS documentation.
115+ context in which they are called. While S3 supports bucket names in the
116+ host name, we use the convention of providing it in the path so that
117+ using IP addresses and alternative implementations of S3 actually works
118+ (e.g. Walrus).
119 """
120 def __init__(self, service_endpoint, bucket="", object_name=""):
121 self.endpoint = service_endpoint
122@@ -43,18 +44,15 @@
123 self.object_name = object_name
124
125 def get_host(self):
126- if not self.bucket:
127- return self.endpoint.get_host()
128- else:
129- return "%s.%s" % (self.bucket, self.endpoint.get_host())
130+ return self.endpoint.get_host()
131
132 def get_path(self):
133 path = "/"
134 if self.bucket is not None and self.object_name:
135- if self.object_name.startswith("/"):
136- path = self.object_name
137- else:
138- path += self.object_name
139+ path += self.bucket
140+ if not self.object_name.startswith("/"):
141+ path += "/"
142+ path += self.object_name
143 return path
144
145 def get_url(self):
146@@ -62,24 +60,6 @@
147 self.endpoint.scheme, self.get_host(), self.get_path())
148
149
150-class BucketURLContext(URLContext):
151- """
152- This URL context class provides a means of overriding the standard
153- behaviour of the URLContext object so that when creating or deleting a
154- bucket, the appropriate URL is obtained.
155-
156- When creating and deleting buckets on AWS, if the host is set as documented
157- (bucketname.s3.amazonaws.com), a 403 error is returned. When, however, one
158- sets the host without the bucket name prefix, the operation is completed
159- successfully.
160- """
161- def get_host(self):
162- return self.endpoint.get_host()
163-
164- def get_path(self):
165- return "/%s" % (self.bucket)
166-
167-
168 class S3Client(BaseClient):
169 """A client for S3."""
170
171@@ -121,8 +101,7 @@
172 query = self.query_factory(
173 action="PUT", creds=self.creds, endpoint=self.endpoint,
174 bucket=bucket)
175- url_context = BucketURLContext(self.endpoint, bucket)
176- return query.submit(url_context)
177+ return query.submit()
178
179 def delete_bucket(self, bucket):
180 """
181@@ -133,8 +112,7 @@
182 query = self.query_factory(
183 action="DELETE", creds=self.creds, endpoint=self.endpoint,
184 bucket=bucket)
185- url_context = BucketURLContext(self.endpoint, bucket)
186- return query.submit(url_context)
187+ return query.submit()
188
189 def get_bucket(self, bucket):
190 """
191@@ -143,8 +121,7 @@
192 query = self.query_factory(
193 action="GET", creds=self.creds, endpoint=self.endpoint,
194 bucket=bucket)
195- url_context = BucketURLContext(self.endpoint, bucket)
196- d = query.submit(url_context)
197+ d = query.submit()
198 return d.addCallback(self._parse_get_bucket)
199
200 def _parse_get_bucket(self, xml_bytes):
201
202=== modified file 'txaws/s3/tests/test_client.py'
203--- txaws/s3/tests/test_client.py 2011-03-26 12:50:41 +0000
204+++ txaws/s3/tests/test_client.py 2011-04-13 14:52:38 +0000
205@@ -26,7 +26,7 @@
206
207 def test_get_host_with_bucket(self):
208 url_context = client.URLContext(self.endpoint, "mystuff")
209- self.assertEquals(url_context.get_host(), "mystuff.s3.amazonaws.com")
210+ self.assertEquals(url_context.get_host(), "s3.amazonaws.com")
211
212 def test_get_path_with_no_bucket(self):
213 url_context = client.URLContext(self.endpoint)
214@@ -39,14 +39,14 @@
215 def test_get_path_with_bucket_and_object(self):
216 url_context = client.URLContext(
217 self.endpoint, bucket="mystuff", object_name="/images/thing.jpg")
218- self.assertEquals(url_context.get_host(), "mystuff.s3.amazonaws.com")
219- self.assertEquals(url_context.get_path(), "/images/thing.jpg")
220+ self.assertEquals(url_context.get_host(), "s3.amazonaws.com")
221+ self.assertEquals(url_context.get_path(), "/mystuff/images/thing.jpg")
222
223 def test_get_path_with_bucket_and_object_without_slash(self):
224 url_context = client.URLContext(
225 self.endpoint, bucket="mystuff", object_name="images/thing.jpg")
226- self.assertEquals(url_context.get_host(), "mystuff.s3.amazonaws.com")
227- self.assertEquals(url_context.get_path(), "/images/thing.jpg")
228+ self.assertEquals(url_context.get_host(), "s3.amazonaws.com")
229+ self.assertEquals(url_context.get_path(), "/mystuff/images/thing.jpg")
230
231 def test_get_url_with_custom_endpoint(self):
232 endpoint = AWSServiceEndpoint("http://localhost/")
233@@ -60,23 +60,11 @@
234 endpoint, bucket="mydocs", object_name="notes.txt")
235 self.assertEquals(
236 url_context.get_url(),
237- "http://mydocs.localhost/notes.txt")
238+ "http://localhost/mydocs/notes.txt")
239
240 URLContextTestCase.skip = s3clientSkip
241
242
243-class BucketURLContextTestCase(TXAWSTestCase):
244-
245- endpoint = AWSServiceEndpoint("https://s3.amazonaws.com/")
246-
247- def test_get_host_with_bucket(self):
248- url_context = client.BucketURLContext(self.endpoint, "mystuff")
249- self.assertEquals(url_context.get_host(), "s3.amazonaws.com")
250- self.assertEquals(url_context.get_path(), "/mystuff")
251-
252-BucketURLContextTestCase.skip = s3clientSkip
253-
254-
255 class S3ClientTestCase(TXAWSTestCase):
256
257 def setUp(self):

Subscribers

People subscribed via source and target branches