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 all commits
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
4 changes: 2 additions & 2 deletions bigquery/google/cloud/bigquery/client.py
Original file line number Diff line number Diff line change
Expand Up @@ -64,10 +64,10 @@ class Client(ClientWithProject):
passed), falls back to the default inferred from the
environment.

:type _http: :class:`~httplib2.Http`
:type _http: :class:`~requests.Session`
:param _http: (Optional) HTTP object to make requests. Can be any object
that defines ``request()`` with the same interface as
:meth:`~httplib2.Http.request`. If not passed, an
:meth:`requests.Session.request`. If not passed, an
``_http`` object is created that is bound to the
``credentials`` for the current object.
This parameter should be considered private, and could
Expand Down
2 changes: 1 addition & 1 deletion bigquery/google/cloud/bigquery/dataset.py
Original file line number Diff line number Diff line change
Expand Up @@ -364,7 +364,7 @@ def _parse_access_grants(access):
def _set_properties(self, api_response):
"""Update properties from resource in body of ``api_response``

:type api_response: httplib2.Response
:type api_response: dict
:param api_response: response returned from an API call.
"""
self._properties.clear()
Expand Down
14 changes: 3 additions & 11 deletions bigquery/google/cloud/bigquery/job.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@

"""Define API Jobs."""

import collections
import threading

import six
Expand Down Expand Up @@ -58,8 +57,6 @@
'tableUnavailable': http_client.BAD_REQUEST,
}

_FakeResponse = collections.namedtuple('_FakeResponse', ['status'])


def _error_result_to_exception(error_result):
"""Maps BigQuery error reasons to an exception.
Expand All @@ -79,13 +76,8 @@ def _error_result_to_exception(error_result):
reason = error_result.get('reason')
status_code = _ERROR_REASON_TO_EXCEPTION.get(
reason, http_client.INTERNAL_SERVER_ERROR)
# make_exception expects an httplib2 response object.
fake_response = _FakeResponse(status=status_code)
return exceptions.make_exception(
fake_response,
error_result.get('message', ''),
error_info=error_result,
use_json=False)
return exceptions.from_http_status(

This comment was marked as spam.

status_code, error_result.get('message', ''), errors=[error_result])


class Compression(_EnumProperty):
Expand Down Expand Up @@ -307,7 +299,7 @@ def _scrub_local_properties(self, cleaned):
def _set_properties(self, api_response):
"""Update properties from resource in body of ``api_response``

:type api_response: httplib2.Response
:type api_response: dict
:param api_response: response returned from an API call
"""
cleaned = api_response.copy()
Expand Down
2 changes: 1 addition & 1 deletion bigquery/google/cloud/bigquery/query.py
Original file line number Diff line number Diff line change
Expand Up @@ -310,7 +310,7 @@ def schema(self):
def _set_properties(self, api_response):
"""Update properties from resource in body of ``api_response``

:type api_response: httplib2.Response
:type api_response: dict
:param api_response: response returned from an API call
"""
self._properties.clear()
Expand Down
29 changes: 4 additions & 25 deletions bigquery/google/cloud/bigquery/table.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,18 +17,16 @@
import datetime
import os

import httplib2
import six

import google.auth.transport.requests
from google import resumable_media
from google.resumable_media.requests import MultipartUpload
from google.resumable_media.requests import ResumableUpload

from google.cloud import exceptions
from google.cloud._helpers import _datetime_from_microseconds
from google.cloud._helpers import _millis_from_datetime
from google.cloud.exceptions import NotFound
from google.cloud.exceptions import make_exception
from google.cloud.iterator import HTTPIterator
from google.cloud.bigquery.schema import SchemaField
from google.cloud.bigquery._helpers import _item_to_row
Expand Down Expand Up @@ -474,7 +472,7 @@ def _require_client(self, client):
def _set_properties(self, api_response):
"""Update properties from resource in body of ``api_response``

:type api_response: httplib2.Response
:type api_response: dict
:param api_response: response returned from an API call
"""
self._properties.clear()
Expand Down Expand Up @@ -563,7 +561,7 @@ def exists(self, client=None):
try:
client._connection.api_request(method='GET', path=self.path,
query_params={'fields': 'id'})
except NotFound:
except exceptions.NotFound:
return False
else:
return True
Expand Down Expand Up @@ -1113,7 +1111,7 @@ def upload_from_file(self,
client, file_obj, metadata, size, num_retries)
return client.job_from_resource(created_json)
except resumable_media.InvalidResponse as exc:
_raise_from_invalid_response(exc)
raise exceptions.from_http_response(exc.response)
# pylint: enable=too-many-arguments,too-many-locals


Expand Down Expand Up @@ -1298,22 +1296,3 @@ def _get_upload_metadata(source_format, schema, dataset, name):
'load': load_config,
},
}


def _raise_from_invalid_response(error, error_info=None):
"""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)
14 changes: 8 additions & 6 deletions bigquery/tests/unit/test__http.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
import unittest

import mock
import requests


class TestConnection(unittest.TestCase):
Expand Down Expand Up @@ -55,10 +56,12 @@ def test_extra_headers(self):
from google.cloud import _http as base_http
from google.cloud.bigquery 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 @@ -68,15 +71,14 @@ 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,
)
2 changes: 1 addition & 1 deletion bigquery/tests/unit/test_job.py
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ def test_simple(self):
exception = self._call_fut(error_result)
self.assertEqual(exception.code, http_client.BAD_REQUEST)
self.assertTrue(exception.message.startswith('bad request'))
self.assertIn("'reason': 'invalid'", exception.message)
self.assertIn(error_result, exception.errors)

def test_missing_reason(self):
error_result = {}
Expand Down
10 changes: 6 additions & 4 deletions bigquery/tests/unit/test_table.py
Original file line number Diff line number Diff line change
Expand Up @@ -1744,9 +1744,11 @@ def _make_table():
def _make_response(status_code, content='', headers={}):
"""Make a mock HTTP response."""
import requests
response = mock.create_autospec(requests.Response, instance=True)
response.content = content.encode('utf-8')
response.headers = headers
response = requests.Response()
response.request = requests.Request(
'POST', 'http://example.com').prepare()
response._content = content.encode('utf-8')
response.headers.update(headers)
response.status_code = status_code
return response

Expand Down Expand Up @@ -1921,7 +1923,7 @@ def test_upload_from_file_failure(self):
table.upload_from_file(
file_obj, source_format='CSV', rewind=True)

assert exc_info.value.message == response.content.decode('utf-8')
assert response.text in exc_info.value.message
assert exc_info.value.errors == []

def test_upload_from_file_bad_mode(self):
Expand Down
5 changes: 2 additions & 3 deletions core/google/cloud/_helpers.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,12 +25,11 @@
import re
from threading import local as Local

import google_auth_httplib2
import httplib2
import six
from six.moves import http_client

import google.auth
import google.auth.transport.requests
from google.protobuf import duration_pb2
from google.protobuf import timestamp_pb2

Expand Down Expand Up @@ -550,7 +549,7 @@ def make_secure_channel(credentials, user_agent, host, extra_options=()):
:returns: gRPC secure channel with credentials attached.
"""
target = '%s:%d' % (host, http_client.HTTPS_PORT)
http_request = google_auth_httplib2.Request(http=httplib2.Http())
http_request = google.auth.transport.requests.Request()

This comment was marked as spam.

This comment was marked as spam.


user_agent_option = ('grpc.primary_user_agent', user_agent)
options = (user_agent_option,) + extra_options
Expand Down
54 changes: 18 additions & 36 deletions core/google/cloud/_http.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,9 @@
import platform

from pkg_resources import get_distribution
import six
from six.moves.urllib.parse import urlencode

from google.cloud.exceptions import make_exception
from google.cloud import exceptions


API_BASE_URL = 'https://www.googleapis.com'
Expand Down Expand Up @@ -67,8 +66,9 @@ def credentials(self):
def http(self):
"""A getter for the HTTP transport used in talking to the API.

:rtype: :class:`httplib2.Http`
:returns: A Http object used to transport data.
Returns:
google.auth.transport.requests.AuthorizedSession:
A :class:`requests.Session` instance.
"""
return self._client._http

Expand Down Expand Up @@ -168,23 +168,13 @@ def _make_request(self, method, url, data=None, content_type=None,
custom behavior, for example, to defer an HTTP request and complete
initialization of the object at a later time.

:rtype: tuple of ``response`` (a dictionary of sorts)
and ``content`` (a string).
:returns: The HTTP response object and the content of the response,
returned by :meth:`_do_request`.
:rtype: :class:`requests.Response`
:returns: The HTTP response.
"""
headers = headers or {}
headers.update(self._EXTRA_HEADERS)
headers['Accept-Encoding'] = 'gzip'

if data:
content_length = len(str(data))
else:
content_length = 0

# NOTE: str is intended, bytes are sufficient for headers.
headers['Content-Length'] = str(content_length)

if content_type:
headers['Content-Type'] = content_type

Expand Down Expand Up @@ -215,12 +205,11 @@ def _do_request(self, method, url, headers, data,
(Optional) Unused ``target_object`` here but may be used by a
superclass.

:rtype: tuple of ``response`` (a dictionary of sorts)
and ``content`` (a string).
:returns: The HTTP response object and the content of the response.
:rtype: :class:`requests.Response`
:returns: The HTTP response.
"""
return self.http.request(uri=url, method=method, headers=headers,
body=data)
return self.http.request(
url=url, method=method, headers=headers, data=data)

def api_request(self, method, path, query_params=None,
data=None, content_type=None, headers=None,
Expand Down Expand Up @@ -281,7 +270,7 @@ def api_request(self, method, path, query_params=None,

:raises ~google.cloud.exceptions.GoogleCloudError: if the response code
is not 200 OK.
:raises TypeError: if the response content type is not JSON.
:raises ValueError: if the response content type is not JSON.
:rtype: dict or str
:returns: The API response payload, either as a raw string or
a dictionary if the response is valid JSON.
Expand All @@ -296,21 +285,14 @@ def api_request(self, method, path, query_params=None,
data = json.dumps(data)
content_type = 'application/json'

response, content = self._make_request(
response = self._make_request(
method=method, url=url, data=data, content_type=content_type,
headers=headers, target_object=_target_object)

if not 200 <= response.status < 300:
raise make_exception(response, content,
error_info=method + ' ' + url)
if not 200 <= response.status_code < 300:
raise exceptions.from_http_response(response)

string_or_bytes = (six.binary_type, six.text_type)
if content and expect_json and isinstance(content, string_or_bytes):
content_type = response.get('content-type', '')
if not content_type.startswith('application/json'):
raise TypeError('Expected JSON, got %s' % content_type)
if isinstance(content, six.binary_type):
content = content.decode('utf-8')
return json.loads(content)

This comment was marked as spam.


return content
if expect_json and response.content:
return response.json()
else:
return response.content
Loading