Skip to content

Commit 53f6b86

Browse files
dhermesdhermes
committed
Small tweaks to Connection in storage; towards a cleaner API.
- Making Connection.make_request non-public - Making Connection.build_api_url a class method
1 parent d858ff0 commit 53f6b86

File tree

4 files changed

+75
-46
lines changed

4 files changed

+75
-46
lines changed

‎gcloud/storage/blob.py‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -306,14 +306,15 @@ def upload_from_file(self, file_obj, rewind=False, size=None,
306306

307307
# Temporary URL, until we know simple vs. resumable.
308308
upload_url = conn.build_api_url(
309-
path=self.bucket.path + '/o', upload=True)
309+
project=conn.project, path=self.bucket.path + '/o', upload=True)
310310

311311
# Use apitools 'Upload' facility.
312312
request = http_wrapper.Request(upload_url, 'POST', headers)
313313

314314
upload.ConfigureRequest(upload_config, request, url_builder)
315315
query_params = url_builder.query_params
316-
request.url = conn.build_api_url(path=self.bucket.path + '/o',
316+
request.url = conn.build_api_url(project=conn.project,
317+
path=self.bucket.path + '/o',
317318
query_params=query_params,
318319
upload=True)
319320
upload.InitializeUpload(request, conn.http)

‎gcloud/storage/connection.py‎

Lines changed: 18 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -147,6 +147,9 @@ class Connection(_Base):
147147
148148
>>> print 'my-bucket-name' in connection
149149
True
150+
151+
:type project: string
152+
:param project: The project name to connect to.
150153
"""
151154

152155
API_VERSION = 'v1'
@@ -158,10 +161,6 @@ class Connection(_Base):
158161
API_ACCESS_ENDPOINT = 'https://storage.googleapis.com'
159162

160163
def __init__(self, project, *args, **kwargs):
161-
""":type project: string
162-
163-
:param project: The project name to connect to.
164-
"""
165164
super(Connection, self).__init__(*args, **kwargs)
166165
self.project = project
167166

@@ -171,12 +170,16 @@ def __iter__(self):
171170
def __contains__(self, bucket_name):
172171
return self.lookup(bucket_name) is not None
173172

174-
def build_api_url(self, path, query_params=None, api_base_url=None,
173+
@classmethod
174+
def build_api_url(cls, project, path, query_params=None, api_base_url=None,
175175
api_version=None, upload=False):
176176
"""Construct an API url given a few components, some optional.
177177
178178
Typically, you shouldn't need to use this method.
179179
180+
:type project: string
181+
:param project: The project name to connect to.
182+
180183
:type path: string
181184
:param path: The path to the resource (ie, ``'/b/bucket-name'``).
182185
@@ -199,23 +202,23 @@ def build_api_url(self, path, query_params=None, api_base_url=None,
199202
:rtype: string
200203
:returns: The URL assembled from the pieces provided.
201204
"""
202-
api_base_url = api_base_url or self.API_BASE_URL
205+
api_base_url = api_base_url or cls.API_BASE_URL
203206
if upload:
204207
api_base_url += '/upload'
205208

206-
url = self.API_URL_TEMPLATE.format(
207-
api_base_url=(api_base_url or self.API_BASE_URL),
208-
api_version=(api_version or self.API_VERSION),
209+
url = cls.API_URL_TEMPLATE.format(
210+
api_base_url=(api_base_url or cls.API_BASE_URL),
211+
api_version=(api_version or cls.API_VERSION),
209212
path=path)
210213

211214
query_params = query_params or {}
212-
query_params.update({'project': self.project})
215+
query_params.update({'project': project})
213216
url += '?' + urllib.urlencode(query_params)
214217

215218
return url
216219

217-
def make_request(self, method, url, data=None, content_type=None,
218-
headers=None):
220+
def _make_request(self, method, url, data=None, content_type=None,
221+
headers=None):
219222
"""A low level method to send a request to the API.
220223
221224
Typically, you shouldn't need to use this method.
@@ -307,7 +310,8 @@ def api_request(self, method, path, query_params=None,
307310
308311
:raises: Exception if the response code is not 200 OK.
309312
"""
310-
url = self.build_api_url(path=path, query_params=query_params,
313+
url = self.build_api_url(project=self.project, path=path,
314+
query_params=query_params,
311315
api_base_url=api_base_url,
312316
api_version=api_version)
313317

@@ -317,7 +321,7 @@ def api_request(self, method, path, query_params=None,
317321
data = json.dumps(data)
318322
content_type = 'application/json'
319323

320-
response, content = self.make_request(
324+
response, content = self._make_request(
321325
method=method, url=url, data=data, content_type=content_type)
322326

323327
if not 200 <= response.status < 300:

‎gcloud/storage/test_blob.py‎

Lines changed: 35 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -291,8 +291,12 @@ def test_upload_from_file_simple(self):
291291
self.assertEqual(scheme, 'http')
292292
self.assertEqual(netloc, 'example.com')
293293
self.assertEqual(path, '/b/name/o')
294-
self.assertEqual(dict(parse_qsl(qs)),
295-
{'uploadType': 'media', 'name': BLOB_NAME})
294+
query_params = {
295+
'uploadType': 'media',
296+
'name': BLOB_NAME,
297+
'project': 'PROJECT',
298+
}
299+
self.assertEqual(dict(parse_qsl(qs)), query_params)
296300
headers = dict(
297301
[(x.title(), str(y)) for x, y in rq[0]['headers'].items()])
298302
self.assertEqual(headers['Content-Length'], '6')
@@ -335,8 +339,12 @@ def test_upload_from_file_resumable(self):
335339
self.assertEqual(scheme, 'http')
336340
self.assertEqual(netloc, 'example.com')
337341
self.assertEqual(path, '/b/name/o')
338-
self.assertEqual(dict(parse_qsl(qs)),
339-
{'uploadType': 'resumable', 'name': BLOB_NAME})
342+
query_params = {
343+
'uploadType': 'resumable',
344+
'name': BLOB_NAME,
345+
'project': 'PROJECT',
346+
}
347+
self.assertEqual(dict(parse_qsl(qs)), query_params)
340348
headers = dict(
341349
[(x.title(), str(y)) for x, y in rq[0]['headers'].items()])
342350
self.assertEqual(headers['X-Upload-Content-Length'], '6')
@@ -390,8 +398,12 @@ def test_upload_from_file_w_slash_in_name(self):
390398
self.assertEqual(scheme, 'http')
391399
self.assertEqual(netloc, 'example.com')
392400
self.assertEqual(path, '/b/name/o')
393-
self.assertEqual(dict(parse_qsl(qs)),
394-
{'uploadType': 'media', 'name': 'parent/child'})
401+
query_params = {
402+
'uploadType': 'media',
403+
'name': 'parent/child',
404+
'project': 'PROJECT',
405+
}
406+
self.assertEqual(dict(parse_qsl(qs)), query_params)
395407
headers = dict(
396408
[(x.title(), str(y)) for x, y in rq[0]['headers'].items()])
397409
self.assertEqual(headers['Content-Length'], '6')
@@ -430,8 +442,12 @@ def test_upload_from_filename(self):
430442
self.assertEqual(scheme, 'http')
431443
self.assertEqual(netloc, 'example.com')
432444
self.assertEqual(path, '/b/name/o')
433-
self.assertEqual(dict(parse_qsl(qs)),
434-
{'uploadType': 'media', 'name': BLOB_NAME})
445+
query_params = {
446+
'uploadType': 'media',
447+
'name': BLOB_NAME,
448+
'project': 'PROJECT',
449+
}
450+
self.assertEqual(dict(parse_qsl(qs)), query_params)
435451
headers = dict(
436452
[(x.title(), str(y)) for x, y in rq[0]['headers'].items()])
437453
self.assertEqual(headers['Content-Length'], '6')
@@ -466,8 +482,12 @@ def test_upload_from_string(self):
466482
self.assertEqual(scheme, 'http')
467483
self.assertEqual(netloc, 'example.com')
468484
self.assertEqual(path, '/b/name/o')
469-
self.assertEqual(dict(parse_qsl(qs)),
470-
{'uploadType': 'media', 'name': BLOB_NAME})
485+
query_params = {
486+
'project': 'PROJECT',
487+
'uploadType': 'media',
488+
'name': BLOB_NAME,
489+
}
490+
self.assertEqual(dict(parse_qsl(qs)), query_params)
471491
headers = dict(
472492
[(x.title(), str(y)) for x, y in rq[0]['headers'].items()])
473493
self.assertEqual(headers['Content-Length'], '6')
@@ -825,6 +845,7 @@ class _Connection(_Responder):
825845

826846
API_BASE_URL = 'http://example.com'
827847
USER_AGENT = 'testing 1.2.3'
848+
project = 'PROJECT'
828849

829850
def __init__(self, *responses):
830851
super(_Connection, self).__init__(*responses)
@@ -834,15 +855,17 @@ def __init__(self, *responses):
834855
def api_request(self, **kw):
835856
return self._respond(**kw)
836857

837-
def build_api_url(self, path, query_params=None,
858+
def build_api_url(self, project, path, query_params=None,
838859
api_base_url=API_BASE_URL, upload=False):
839860
from urllib import urlencode
840861
from urlparse import urlsplit
841862
from urlparse import urlunsplit
842863
# mimic the build_api_url interface, but avoid unused param and
843864
# missed coverage errors
844865
upload = not upload # pragma NO COVER
845-
qs = urlencode(query_params or {})
866+
query_params = query_params or {}
867+
query_params['project'] = project
868+
qs = urlencode(query_params)
846869
scheme, netloc, _, _, _ = urlsplit(api_base_url)
847870
return urlunsplit((scheme, netloc, path, qs, ''))
848871

‎gcloud/storage/test_connection.py‎

Lines changed: 19 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -141,50 +141,51 @@ def test___contains___hit(self):
141141

142142
def test_build_api_url_no_extra_query_params(self):
143143
PROJECT = 'project'
144-
conn = self._makeOne(PROJECT)
144+
klass = self._getTargetClass()
145145
URI = '/'.join([
146-
conn.API_BASE_URL,
146+
klass.API_BASE_URL,
147147
'storage',
148-
conn.API_VERSION,
148+
klass.API_VERSION,
149149
'foo?project=%s' % PROJECT,
150150
])
151-
self.assertEqual(conn.build_api_url('/foo'), URI)
151+
self.assertEqual(klass.build_api_url(PROJECT, '/foo'), URI)
152152

153153
def test_build_api_url_w_extra_query_params(self):
154154
from urlparse import parse_qsl
155155
from urlparse import urlsplit
156156
PROJECT = 'project'
157-
conn = self._makeOne(PROJECT)
158-
uri = conn.build_api_url('/foo', {'bar': 'baz'})
157+
klass = self._getTargetClass()
158+
uri = klass.build_api_url(PROJECT, '/foo', query_params={'bar': 'baz'})
159159
scheme, netloc, path, qs, _ = urlsplit(uri)
160-
self.assertEqual('%s://%s' % (scheme, netloc), conn.API_BASE_URL)
160+
self.assertEqual('%s://%s' % (scheme, netloc), klass.API_BASE_URL)
161161
self.assertEqual(path,
162-
'/'.join(['', 'storage', conn.API_VERSION, 'foo']))
162+
'/'.join(['', 'storage', klass.API_VERSION, 'foo']))
163163
parms = dict(parse_qsl(qs))
164164
self.assertEqual(parms['project'], PROJECT)
165165
self.assertEqual(parms['bar'], 'baz')
166166

167167
def test_build_api_url_w_upload(self):
168168
PROJECT = 'project'
169-
conn = self._makeOne(PROJECT)
169+
klass = self._getTargetClass()
170170
URI = '/'.join([
171-
conn.API_BASE_URL,
171+
klass.API_BASE_URL,
172172
'upload',
173173
'storage',
174-
conn.API_VERSION,
174+
klass.API_VERSION,
175175
'foo?project=%s' % PROJECT,
176176
])
177-
self.assertEqual(conn.build_api_url('/foo', upload=True), URI)
177+
self.assertEqual(klass.build_api_url(PROJECT, '/foo', upload=True),
178+
URI)
178179

179-
def test_make_request_no_data_no_content_type_no_headers(self):
180+
def test__make_request_no_data_no_content_type_no_headers(self):
180181
PROJECT = 'project'
181182
conn = self._makeOne(PROJECT)
182183
URI = 'http://example.com/test'
183184
http = conn._http = Http(
184185
{'status': '200', 'content-type': 'text/plain'},
185186
'',
186187
)
187-
headers, content = conn.make_request('GET', URI)
188+
headers, content = conn._make_request('GET', URI)
188189
self.assertEqual(headers['status'], '200')
189190
self.assertEqual(headers['content-type'], 'text/plain')
190191
self.assertEqual(content, '')
@@ -198,15 +199,15 @@ def test_make_request_no_data_no_content_type_no_headers(self):
198199
}
199200
self.assertEqual(http._called_with['headers'], expected_headers)
200201

201-
def test_make_request_w_data_no_extra_headers(self):
202+
def test__make_request_w_data_no_extra_headers(self):
202203
PROJECT = 'project'
203204
conn = self._makeOne(PROJECT)
204205
URI = 'http://example.com/test'
205206
http = conn._http = Http(
206207
{'status': '200', 'content-type': 'text/plain'},
207208
'',
208209
)
209-
conn.make_request('GET', URI, {}, 'application/json')
210+
conn._make_request('GET', URI, {}, 'application/json')
210211
self.assertEqual(http._called_with['method'], 'GET')
211212
self.assertEqual(http._called_with['uri'], URI)
212213
self.assertEqual(http._called_with['body'], {})
@@ -218,15 +219,15 @@ def test_make_request_w_data_no_extra_headers(self):
218219
}
219220
self.assertEqual(http._called_with['headers'], expected_headers)
220221

221-
def test_make_request_w_extra_headers(self):
222+
def test__make_request_w_extra_headers(self):
222223
PROJECT = 'project'
223224
conn = self._makeOne(PROJECT)
224225
URI = 'http://example.com/test'
225226
http = conn._http = Http(
226227
{'status': '200', 'content-type': 'text/plain'},
227228
'',
228229
)
229-
conn.make_request('GET', URI, headers={'X-Foo': 'foo'})
230+
conn._make_request('GET', URI, headers={'X-Foo': 'foo'})
230231
self.assertEqual(http._called_with['method'], 'GET')
231232
self.assertEqual(http._called_with['uri'], URI)
232233
self.assertEqual(http._called_with['body'], None)

0 commit comments

Comments
 (0)