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

Remove httplib2, replace with Requests #3674

Merged
merged 22 commits into from
Jul 27, 2017
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
82 changes: 46 additions & 36 deletions storage/google/cloud/storage/batch.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,10 +23,10 @@
import io
import json

import httplib2
import requests
import six

from google.cloud.exceptions import make_exception
from google.cloud import exceptions
from google.cloud.storage._http import Connection


Expand Down Expand Up @@ -70,11 +70,6 @@ def __init__(self, method, uri, headers, body):
super_init(payload, 'http', encode_noop)


class NoContent(object):
"""Emulate an HTTP '204 No Content' response."""
status = 204


class _FutureDict(object):
"""Class to hold a future value for a deferred request.

Expand Down Expand Up @@ -123,6 +118,19 @@ def __setitem__(self, key, value):
raise KeyError('Cannot set %r -> %r on a future' % (key, value))


class _FutureResponse(requests.Response):
def __init__(self, future_dict):
super(_FutureResponse, self).__init__()
self._future_dict = future_dict
self.status_code = 204

def json(self):
raise ValueError()

def content(self):
return self._future_dict


class Batch(Connection):
"""Proxy an underlying connection, batching up change operations.

Expand Down Expand Up @@ -171,7 +179,7 @@ def _do_request(self, method, url, headers, data, target_object):
self._target_objects.append(target_object)
if target_object is not None:
target_object._properties = result
return NoContent(), result
return _FutureResponse(result)

def _prepare_batch_request(self):
"""Prepares headers and body for a batch request.
Expand Down Expand Up @@ -218,17 +226,18 @@ def _finish_futures(self, responses):
if len(self._target_objects) != len(responses):
raise ValueError('Expected a response for every request.')

for target_object, sub_response in zip(self._target_objects,
responses):
resp_headers, sub_payload = sub_response
if not 200 <= resp_headers.status < 300:
exception_args = exception_args or (resp_headers,
sub_payload)
for target_object, subresponse in zip(
self._target_objects, responses):
if not 200 <= subresponse.status_code < 300:
exception_args = exception_args or subresponse
elif target_object is not None:
target_object._properties = sub_payload
try:
target_object._properties = subresponse.json()
except ValueError:
target_object._properties = subresponse.content

if exception_args is not None:
raise make_exception(*exception_args)
raise exceptions.from_http_response(exception_args)

def finish(self):
"""Submit a single `multipart/mixed` request with deferred requests.
Expand All @@ -243,9 +252,9 @@ def finish(self):
# Use the private ``_base_connection`` rather than the property
# ``_connection``, since the property may be this
# current batch.
response, content = self._client._base_connection._make_request(
response = self._client._base_connection._make_request(
'POST', url, data=body, headers=headers)
responses = list(_unpack_batch_response(response, content))
responses = list(_unpack_batch_response(response))
self._finish_futures(responses)
return responses

Expand All @@ -265,24 +274,23 @@ def __exit__(self, exc_type, exc_val, exc_tb):
self._client._pop_batch()


def _generate_faux_mime_message(parser, response, content):
def _generate_faux_mime_message(parser, response):
"""Convert response, content -> (multipart) email.message.

Helper for _unpack_batch_response.
"""
# We coerce to bytes to get consistent concat across
# Py2 and Py3. Percent formatting is insufficient since
# it includes the b in Py3.
if not isinstance(content, six.binary_type):
content = content.encode('utf-8')
content_type = response['content-type']
content_type = response.headers.get('content-type', '')
if not isinstance(content_type, six.binary_type):
content_type = content_type.encode('utf-8')

This comment was marked as spam.

This comment was marked as spam.


faux_message = b''.join([
b'Content-Type: ',
content_type,
b'\nMIME-Version: 1.0\n\n',
content,
response.content,

This comment was marked as spam.

This comment was marked as spam.

])

if six.PY2:
Expand All @@ -291,20 +299,17 @@ def _generate_faux_mime_message(parser, response, content):
return parser.parsestr(faux_message.decode('utf-8'))


def _unpack_batch_response(response, content):
"""Convert response, content -> [(headers, payload)].
def _unpack_batch_response(response):
"""Convert requests.Response -> [(headers, payload)].

Creates a generator of tuples of emulating the responses to
:meth:`httplib2.Http.request` (a pair of headers and payload).

:type response: :class:`httplib2.Response`
:type response: :class:`requests.Response`
:param response: HTTP response / headers from a request.

:type content: str
:param content: Response payload with a batch response.
"""
parser = Parser()
message = _generate_faux_mime_message(parser, response, content)
message = _generate_faux_mime_message(parser, response)

if not isinstance(message._payload, list):
raise ValueError('Bad response: not multi-part')
Expand All @@ -314,10 +319,15 @@ def _unpack_batch_response(response, content):
_, status, _ = status_line.split(' ', 2)
sub_message = parser.parsestr(rest)
payload = sub_message._payload
ctype = sub_message['Content-Type']
msg_headers = dict(sub_message._headers)
msg_headers['status'] = status
headers = httplib2.Response(msg_headers)
if ctype and ctype.startswith('application/json'):
payload = json.loads(payload)
yield headers, payload
content_id = msg_headers.get('Content-ID')

subresponse = requests.Response()
subresponse.request = requests.Request(
method='BATCH',
url='contentid://{}'.format(content_id)).prepare()

This comment was marked as spam.

This comment was marked as spam.

subresponse.status_code = int(status)
subresponse.headers.update(msg_headers)
subresponse._content = payload.encode('utf-8')

yield subresponse
15 changes: 4 additions & 11 deletions storage/google/cloud/storage/blob.py
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@
import time
import warnings

import httplib2
from six.moves.urllib.parse import quote

import google.auth.transport.requests
Expand All @@ -44,11 +43,11 @@
from google.resumable_media.requests import MultipartUpload
from google.resumable_media.requests import ResumableUpload

from google.cloud import exceptions
from google.cloud._helpers import _rfc3339_to_datetime
from google.cloud._helpers import _to_bytes
from google.cloud._helpers import _bytes_to_unicode
from google.cloud.exceptions import NotFound
from google.cloud.exceptions import make_exception
from google.cloud.iam import Policy
from google.cloud.storage._helpers import _PropertyMixin
from google.cloud.storage._helpers import _scalar_property
Expand Down Expand Up @@ -469,7 +468,7 @@ def download_to_file(self, file_obj, client=None):
try:
self._do_download(transport, file_obj, download_url, headers)
except resumable_media.InvalidResponse as exc:
_raise_from_invalid_response(exc, download_url)
_raise_from_invalid_response(exc)

def download_to_filename(self, filename, client=None):
"""Download the contents of this blob into a named file.
Expand Down Expand Up @@ -1598,20 +1597,14 @@ def _maybe_rewind(stream, rewind=False):
stream.seek(0, os.SEEK_SET)


def _raise_from_invalid_response(error, error_info=None):
def _raise_from_invalid_response(error):
"""Re-wrap and raise an ``InvalidResponse`` exception.

:type error: :exc:`google.resumable_media.InvalidResponse`
:param error: A caught exception from the ``google-resumable-media``
library.

:type error_info: str
:param error_info: (Optional) Extra information about the failed request.

:raises: :class:`~google.cloud.exceptions.GoogleCloudError` corresponding
to the failed status code
"""
response = error.response
faux_response = httplib2.Response({'status': response.status_code})
raise make_exception(faux_response, response.content,
error_info=error_info, use_json=False)
raise exceptions.from_http_response(error.response)
15 changes: 9 additions & 6 deletions storage/tests/unit/test__http.py
Original file line number Diff line number Diff line change
Expand Up @@ -29,13 +29,17 @@ def _make_one(self, *args, **kw):
return self._get_target_class()(*args, **kw)

def test_extra_headers(self):
import requests

from google.cloud import _http as base_http
from google.cloud.storage import _http as MUT

http = mock.Mock(spec=['request'])
response = mock.Mock(status=200, spec=['status'])
http = mock.create_autospec(requests.Session, instance=True)
response = requests.Response()
response.status_code = 200
data = b'brent-spiner'
http.request.return_value = response, data
response._content = data
http.request.return_value = response
client = mock.Mock(_http=http, spec=['_http'])

conn = self._make_one(client)
Expand All @@ -45,17 +49,16 @@ def test_extra_headers(self):
self.assertEqual(result, data)

expected_headers = {
'Content-Length': str(len(req_data)),
'Accept-Encoding': 'gzip',
base_http.CLIENT_INFO_HEADER: MUT._CLIENT_INFO,
'User-Agent': conn.USER_AGENT,
}
expected_uri = conn.build_api_url('/rainbow')
http.request.assert_called_once_with(
body=req_data,
data=req_data,
headers=expected_headers,
method='GET',
uri=expected_uri,
url=expected_uri,
)

def test_build_api_url_no_extra_query_params(self):
Expand Down
Loading