Skip to content

fix api endpoints for storage upload/download #414

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

Merged
silvolu merged 1 commit into from
Dec 12, 2014
Merged

fix api endpoints for storage upload/download #414

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
9 changes: 8 additions & 1 deletion gcloud/storage/connection.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ 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,
api_version=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.
Expand All @@ -112,9 +112,16 @@ def build_api_url(self, path, query_params=None, api_base_url=None,
Typically you shouldn't provide this and instead
use the default for the library.

:type upload: boolean
:param upload: True if the URL is for uploading purposes.

:rtype: string
:returns: The URL assembled from the pieces provided.
"""
api_base_url = api_base_url or self.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),
Expand Down
27 changes: 8 additions & 19 deletions gcloud/storage/key.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -209,13 +209,14 @@ def download_to_file(self, file_obj):

:raises: :class:`gcloud.storage.exceptions.NotFound`
"""

download_url = self.media_link

# Use apitools 'Download' facility.
download = transfer.Download.FromStream(file_obj, auto_transfer=False)
download.chunksize = self.CHUNK_SIZE
download_url = self.connection.build_api_url(
path=self.path, query_params={'alt': 'media'})
headers = {'Range': 'bytes=0-%d' % (self.CHUNK_SIZE - 1)}
request = http_wrapper.Request(download_url, 'POST', headers)
request = http_wrapper.Request(download_url, 'GET', headers)

download.InitializeDownload(request, self.connection.http)

Expand Down Expand Up @@ -308,15 +309,16 @@ 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')
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)
path = url_builder.relative_path.format(bucket=self.bucket.name)
query_params = url_builder.query_params
request.url = conn.build_api_url(path=path, query_params=query_params)
request.url = conn.build_api_url(path=self.bucket.path + '/o',
query_params=query_params,
upload=True)
upload.InitializeUpload(request, conn.http)

# Should we be passing callbacks through from caller? We can't
Expand Down Expand Up @@ -628,16 +630,3 @@ def __init__(self, bucket_name, object_name):
self.query_params = {'name': object_name}
self._bucket_name = bucket_name
self._relative_path = ''

@property
def relative_path(self):
"""Inject bucket name into path."""
return self._relative_path.format(bucket=self._bucket_name)

@relative_path.setter
def relative_path(self, value):
"""Allow update of path template.

``value`` should be a string template taking ``{bucket}``.
"""
self._relative_path = value
12 changes: 12 additions & 0 deletions gcloud/storage/test_connection.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -150,6 +150,18 @@ def test_build_api_url_w_extra_query_params(self):
self.assertEqual(parms['project'], PROJECT)
self.assertEqual(parms['bar'], 'baz')

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

def test_make_request_no_data_no_content_type_no_headers(self):
PROJECT = 'project'
conn = self._makeOne(PROJECT)
Expand Down
27 changes: 18 additions & 9 deletions gcloud/storage/test_key.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -191,7 +191,9 @@ def test_download_to_file(self):
(chunk2_response, 'def'),
)
bucket = _Bucket(connection)
key = self._makeOne(bucket, KEY)
MEDIA_LINK = 'http://example.com/media/'
properties = {'mediaLink': MEDIA_LINK}
key = self._makeOne(bucket, KEY, properties)
key.CHUNK_SIZE = 3
fh = StringIO()
key.download_to_file(fh)
Expand All @@ -210,7 +212,9 @@ def test_download_to_filename(self):
(chunk2_response, 'def'),
)
bucket = _Bucket(connection)
key = self._makeOne(bucket, KEY)
MEDIA_LINK = 'http://example.com/media/'
properties = {'mediaLink': MEDIA_LINK}
key = self._makeOne(bucket, KEY, properties)
key.CHUNK_SIZE = 3
with NamedTemporaryFile() as f:
key.download_to_filename(f.name)
Expand All @@ -231,7 +235,9 @@ def test_download_as_string(self):
(chunk2_response, 'def'),
)
bucket = _Bucket(connection)
key = self._makeOne(bucket, KEY)
MEDIA_LINK = 'http://example.com/media/'
properties = {'mediaLink': MEDIA_LINK}
key = self._makeOne(bucket, KEY, properties)
key.CHUNK_SIZE = 3
fetched = key.download_as_string()
self.assertEqual(fetched, 'abcdef')
Expand Down Expand Up @@ -261,7 +267,7 @@ def test_upload_from_file_simple(self):
scheme, netloc, path, qs, _ = urlsplit(uri)
self.assertEqual(scheme, 'http')
self.assertEqual(netloc, 'example.com')
self.assertEqual(path, '/upload/storage/v1/b/name/o')
self.assertEqual(path, '/b/name/o')
self.assertEqual(dict(parse_qsl(qs)),
{'uploadType': 'media', 'name': 'key'})
headers = dict(
Expand Down Expand Up @@ -305,7 +311,7 @@ def test_upload_from_file_resumable(self):
scheme, netloc, path, qs, _ = urlsplit(uri)
self.assertEqual(scheme, 'http')
self.assertEqual(netloc, 'example.com')
self.assertEqual(path, '/resumable/upload/storage/v1/b/name/o')
self.assertEqual(path, '/b/name/o')
self.assertEqual(dict(parse_qsl(qs)),
{'uploadType': 'resumable', 'name': 'key'})
headers = dict(
Expand Down Expand Up @@ -360,7 +366,7 @@ def test_upload_from_file_w_slash_in_name(self):
scheme, netloc, path, qs, _ = urlsplit(uri)
self.assertEqual(scheme, 'http')
self.assertEqual(netloc, 'example.com')
self.assertEqual(path, '/upload/storage/v1/b/name/o')
self.assertEqual(path, '/b/name/o')
self.assertEqual(dict(parse_qsl(qs)),
{'uploadType': 'media', 'name': 'parent/child'})
headers = dict(
Expand Down Expand Up @@ -400,7 +406,7 @@ def test_upload_from_filename(self):
scheme, netloc, path, qs, _ = urlsplit(uri)
self.assertEqual(scheme, 'http')
self.assertEqual(netloc, 'example.com')
self.assertEqual(path, '/upload/storage/v1/b/name/o')
self.assertEqual(path, '/b/name/o')
self.assertEqual(dict(parse_qsl(qs)),
{'uploadType': 'media', 'name': 'key'})
headers = dict(
Expand Down Expand Up @@ -436,7 +442,7 @@ def test_upload_from_string(self):
scheme, netloc, path, qs, _ = urlsplit(uri)
self.assertEqual(scheme, 'http')
self.assertEqual(netloc, 'example.com')
self.assertEqual(path, '/upload/storage/v1/b/name/o')
self.assertEqual(path, '/b/name/o')
self.assertEqual(dict(parse_qsl(qs)),
{'uploadType': 'media', 'name': 'key'})
headers = dict(
Expand Down Expand Up @@ -806,10 +812,13 @@ def api_request(self, **kw):
return self._respond(**kw)

def build_api_url(self, path, query_params=None,
api_base_url=API_BASE_URL):
api_base_url=API_BASE_URL, upload=False):
from urllib import urlencode
from urlparse import urlsplit
from urlparse import urlunsplit
# mimic the build_api_url interface, but avoid unused param and

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

# missed coverage errors
upload = not upload # pragma NO COVER
qs = urlencode(query_params or {})
scheme, netloc, _, _, _ = urlsplit(api_base_url)
return urlunsplit((scheme, netloc, path, qs, ''))
Expand Down