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.

Sign up for GitHub

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

Jump to bottom

Small tweaks to Connection in storage; towards a cleaner API. #587

Closed
wants to merge 1 commit into from
Closed

Small tweaks to Connection in storage; towards a cleaner API. #587

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions gcloud/storage/blob.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -314,14 +314,15 @@ def upload_from_file(self, file_obj, rewind=False, size=None,

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

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

upload.ConfigureRequest(upload_config, request, url_builder)
query_params = url_builder.query_params
request.url = conn.build_api_url(path=self.bucket.path + '/o',
request.url = conn.build_api_url(project=conn.project,
path=self.bucket.path + '/o',
query_params=query_params,
upload=True)
upload.InitializeUpload(request, conn.http)
Expand Down
32 changes: 18 additions & 14 deletions gcloud/storage/connection.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,9 @@ class Connection(_Base):

>>> print 'my-bucket-name' in connection
True

:type project: string
:param project: The project name to connect to.
"""

API_VERSION = 'v1'
Expand All @@ -75,10 +78,6 @@ class Connection(_Base):
"""A template for the URL of a particular API call."""

def __init__(self, project, *args, **kwargs):
""":type project: string

:param project: The project name to connect to.
"""
super(Connection, self).__init__(*args, **kwargs)
self.project = project

Expand All @@ -88,12 +87,16 @@ def __iter__(self):
def __contains__(self, bucket_name):
return self.lookup(bucket_name) is not None

def build_api_url(self, path, query_params=None, api_base_url=None,
@classmethod
def build_api_url(cls, project, path, query_params=None, api_base_url=None,
api_version=None, upload=False):
"""Construct an API url given a few components, some optional.

Typically, you shouldn't need to use this method.

:type project: string
:param project: The project name to connect to.

:type path: string
:param path: The path to the resource (ie, ``'/b/bucket-name'``).

Expand All @@ -116,23 +119,23 @@ def build_api_url(self, path, query_params=None, api_base_url=None,
:rtype: string
:returns: The URL assembled from the pieces provided.
"""
api_base_url = api_base_url or self.API_BASE_URL
api_base_url = api_base_url or cls.API_BASE_URL
if upload:
api_base_url += '/upload'

url = self.API_URL_TEMPLATE.format(
api_base_url=(api_base_url or self.API_BASE_URL),
api_version=(api_version or self.API_VERSION),
url = cls.API_URL_TEMPLATE.format(
api_base_url=(api_base_url or cls.API_BASE_URL),
api_version=(api_version or cls.API_VERSION),
path=path)

query_params = query_params or {}
query_params.update({'project': self.project})
query_params.update({'project': project})
url += '?' + urlencode(query_params)

return url

def make_request(self, method, url, data=None, content_type=None,
headers=None):
def _make_request(self, method, url, data=None, content_type=None,
headers=None):
"""A low level method to send a request to the API.

Typically, you shouldn't need to use this method.
Expand Down Expand Up @@ -224,7 +227,8 @@ def api_request(self, method, path, query_params=None,

:raises: Exception if the response code is not 200 OK.
"""
url = self.build_api_url(path=path, query_params=query_params,
url = self.build_api_url(project=self.project, path=path,
query_params=query_params,
api_base_url=api_base_url,
api_version=api_version)

Expand All @@ -234,7 +238,7 @@ def api_request(self, method, path, query_params=None,
data = json.dumps(data)
content_type = 'application/json'

response, content = self.make_request(
response, content = self._make_request(
method=method, url=url, data=data, content_type=content_type)

if not 200 <= response.status < 300:
Expand Down
55 changes: 41 additions & 14 deletions gcloud/storage/test_blob.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -332,8 +332,12 @@ def test_upload_from_file_simple(self):
self.assertEqual(scheme, 'http')
self.assertEqual(netloc, 'example.com')
self.assertEqual(path, '/b/name/o')
self.assertEqual(dict(parse_qsl(qs)),
{'uploadType': 'media', 'name': BLOB_NAME})
query_params = {
'uploadType': 'media',
'name': BLOB_NAME,
'project': 'PROJECT',
}
self.assertEqual(dict(parse_qsl(qs)), query_params)
headers = dict(
[(x.title(), str(y)) for x, y in rq[0]['headers'].items()])
self.assertEqual(headers['Content-Length'], '6')
Expand Down Expand Up @@ -376,8 +380,12 @@ def test_upload_from_file_resumable(self):
self.assertEqual(scheme, 'http')
self.assertEqual(netloc, 'example.com')
self.assertEqual(path, '/b/name/o')
self.assertEqual(dict(parse_qsl(qs)),
{'uploadType': 'resumable', 'name': BLOB_NAME})
query_params = {
'uploadType': 'resumable',
'name': BLOB_NAME,
'project': 'PROJECT',
}
self.assertEqual(dict(parse_qsl(qs)), query_params)
headers = dict(
[(x.title(), str(y)) for x, y in rq[0]['headers'].items()])
self.assertEqual(headers['X-Upload-Content-Length'], '6')
Expand Down Expand Up @@ -431,8 +439,12 @@ def test_upload_from_file_w_slash_in_name(self):
self.assertEqual(scheme, 'http')
self.assertEqual(netloc, 'example.com')
self.assertEqual(path, '/b/name/o')
self.assertEqual(dict(parse_qsl(qs)),
{'uploadType': 'media', 'name': 'parent/child'})
query_params = {
'uploadType': 'media',
'name': 'parent/child',
'project': 'PROJECT',
}
self.assertEqual(dict(parse_qsl(qs)), query_params)
headers = dict(
[(x.title(), str(y)) for x, y in rq[0]['headers'].items()])
self.assertEqual(headers['Content-Length'], '6')
Expand Down Expand Up @@ -471,8 +483,12 @@ def test_upload_from_filename(self):
self.assertEqual(scheme, 'http')
self.assertEqual(netloc, 'example.com')
self.assertEqual(path, '/b/name/o')
self.assertEqual(dict(parse_qsl(qs)),
{'uploadType': 'media', 'name': BLOB_NAME})
query_params = {
'uploadType': 'media',
'name': BLOB_NAME,
'project': 'PROJECT',
}
self.assertEqual(dict(parse_qsl(qs)), query_params)
headers = dict(
[(x.title(), str(y)) for x, y in rq[0]['headers'].items()])
self.assertEqual(headers['Content-Length'], '6')
Expand Down Expand Up @@ -507,8 +523,12 @@ def test_upload_from_string_w_bytes(self):
self.assertEqual(scheme, 'http')
self.assertEqual(netloc, 'example.com')
self.assertEqual(path, '/b/name/o')
self.assertEqual(dict(parse_qsl(qs)),
{'uploadType': 'media', 'name': BLOB_NAME})
query_params = {
'project': 'PROJECT',
'uploadType': 'media',
'name': BLOB_NAME,
}
self.assertEqual(dict(parse_qsl(qs)), query_params)
headers = dict(
[(x.title(), str(y)) for x, y in rq[0]['headers'].items()])
self.assertEqual(headers['Content-Length'], '6')
Expand Down Expand Up @@ -545,8 +565,12 @@ def test_upload_from_string_w_text(self):
self.assertEqual(scheme, 'http')
self.assertEqual(netloc, 'example.com')
self.assertEqual(path, '/b/name/o')
self.assertEqual(dict(parse_qsl(qs)),
{'uploadType': 'media', 'name': BLOB_NAME})
query_params = {
'uploadType': 'media',
'name': BLOB_NAME,
'project': 'PROJECT',
}
self.assertEqual(dict(parse_qsl(qs)), query_params)
headers = dict(
[(x.title(), str(y)) for x, y in rq[0]['headers'].items()])
self.assertEqual(headers['Content-Length'], str(len(ENCODED)))
Expand Down Expand Up @@ -906,6 +930,7 @@ class _Connection(_Responder):
API_BASE_URL = 'http://example.com'
USER_AGENT = 'testing 1.2.3'
credentials = object()
project = 'PROJECT'

def __init__(self, *responses):
super(_Connection, self).__init__(*responses)
Expand All @@ -915,15 +940,17 @@ def __init__(self, *responses):
def api_request(self, **kw):
return self._respond(**kw)

def build_api_url(self, path, query_params=None,
def build_api_url(self, project, path, query_params=None,
api_base_url=API_BASE_URL, upload=False):
from six.moves.urllib.parse import urlencode
from six.moves.urllib.parse import urlsplit
from six.moves.urllib.parse import urlunsplit
# mimic the build_api_url interface, but avoid unused param and
# missed coverage errors
upload = not upload # pragma NO COVER
qs = urlencode(query_params or {})
query_params = query_params or {}
query_params['project'] = project
qs = urlencode(query_params)
scheme, netloc, _, _, _ = urlsplit(api_base_url)
return urlunsplit((scheme, netloc, path, qs, ''))

Expand Down
37 changes: 19 additions & 18 deletions gcloud/storage/test_connection.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -141,50 +141,51 @@ def test___contains___hit(self):

def test_build_api_url_no_extra_query_params(self):
PROJECT = 'project'
conn = self._makeOne(PROJECT)
klass = self._getTargetClass()
URI = '/'.join([
conn.API_BASE_URL,
klass.API_BASE_URL,
'storage',
conn.API_VERSION,
klass.API_VERSION,
'foo?project=%s' % PROJECT,
])
self.assertEqual(conn.build_api_url('/foo'), URI)
self.assertEqual(klass.build_api_url(PROJECT, '/foo'), URI)

def test_build_api_url_w_extra_query_params(self):
from six.moves.urllib.parse import parse_qsl
from six.moves.urllib.parse import urlsplit
PROJECT = 'project'
conn = self._makeOne(PROJECT)
uri = conn.build_api_url('/foo', {'bar': 'baz'})
klass = self._getTargetClass()
uri = klass.build_api_url(PROJECT, '/foo', query_params={'bar': 'baz'})
scheme, netloc, path, qs, _ = urlsplit(uri)
self.assertEqual('%s://%s' % (scheme, netloc), conn.API_BASE_URL)
self.assertEqual('%s://%s' % (scheme, netloc), klass.API_BASE_URL)
self.assertEqual(path,
'/'.join(['', 'storage', conn.API_VERSION, 'foo']))
'/'.join(['', 'storage', klass.API_VERSION, 'foo']))
parms = dict(parse_qsl(qs))
self.assertEqual(parms['project'], PROJECT)
self.assertEqual(parms['bar'], 'baz')

def test_build_api_url_w_upload(self):
PROJECT = 'project'
conn = self._makeOne(PROJECT)
klass = self._getTargetClass()
URI = '/'.join([
conn.API_BASE_URL,
klass.API_BASE_URL,
'upload',
'storage',
conn.API_VERSION,
klass.API_VERSION,
'foo?project=%s' % PROJECT,
])
self.assertEqual(conn.build_api_url('/foo', upload=True), URI)
self.assertEqual(klass.build_api_url(PROJECT, '/foo', upload=True),
URI)

def test_make_request_no_data_no_content_type_no_headers(self):
def test__make_request_no_data_no_content_type_no_headers(self):
PROJECT = 'project'
conn = self._makeOne(PROJECT)
URI = 'http://example.com/test'
http = conn._http = Http(
{'status': '200', 'content-type': 'text/plain'},
'',
)
headers, content = conn.make_request('GET', URI)
headers, content = conn._make_request('GET', URI)
self.assertEqual(headers['status'], '200')
self.assertEqual(headers['content-type'], 'text/plain')
self.assertEqual(content, '')
Expand All @@ -198,15 +199,15 @@ def test_make_request_no_data_no_content_type_no_headers(self):
}
self.assertEqual(http._called_with['headers'], expected_headers)

def test_make_request_w_data_no_extra_headers(self):
def test__make_request_w_data_no_extra_headers(self):
PROJECT = 'project'
conn = self._makeOne(PROJECT)
URI = 'http://example.com/test'
http = conn._http = Http(
{'status': '200', 'content-type': 'text/plain'},
'',
)
conn.make_request('GET', URI, {}, 'application/json')
conn._make_request('GET', URI, {}, 'application/json')
self.assertEqual(http._called_with['method'], 'GET')
self.assertEqual(http._called_with['uri'], URI)
self.assertEqual(http._called_with['body'], {})
Expand All @@ -218,15 +219,15 @@ def test_make_request_w_data_no_extra_headers(self):
}
self.assertEqual(http._called_with['headers'], expected_headers)

def test_make_request_w_extra_headers(self):
def test__make_request_w_extra_headers(self):
PROJECT = 'project'
conn = self._makeOne(PROJECT)
URI = 'http://example.com/test'
http = conn._http = Http(
{'status': '200', 'content-type': 'text/plain'},
'',
)
conn.make_request('GET', URI, headers={'X-Foo': 'foo'})
conn._make_request('GET', URI, headers={'X-Foo': 'foo'})
self.assertEqual(http._called_with['method'], 'GET')
self.assertEqual(http._called_with['uri'], URI)
self.assertEqual(http._called_with['body'], None)
Expand Down