Skip to content

Commit

Permalink
Set default HTTP timeout of 60s (googleapis#320)
Browse files Browse the repository at this point in the history
  • Loading branch information
i-maravic authored and Jon Wayne Parrott committed Jan 19, 2017
1 parent 811d570 commit 2243529
Show file tree
Hide file tree
Showing 8 changed files with 105 additions and 23 deletions.
7 changes: 4 additions & 3 deletions describe.py
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@
from googleapiclient.discovery import build
from googleapiclient.discovery import build_from_document
from googleapiclient.discovery import UnknownApiNameOrVersion
import httplib2
from googleapiclient.http import build_http
import uritemplate

CSS = """<style>
Expand Down Expand Up @@ -346,6 +346,7 @@ def document_api(name, version):
print 'Warning: {} {} found but could not be built.'.format(name, version)
return

http = build_http()
response, content = http.request(
uritemplate.expand(
FLAGS.discovery_uri_template, {
Expand All @@ -366,7 +367,7 @@ def document_api_from_discovery_document(uri):
Args:
uri: string, URI of discovery document.
"""
http = httplib2.Http()
http = build_http()
response, content = http.request(FLAGS.discovery_uri)
discovery = json.loads(content)

Expand All @@ -384,7 +385,7 @@ def document_api_from_discovery_document(uri):
if FLAGS.discovery_uri:
document_api_from_discovery_document(FLAGS.discovery_uri)
else:
http = httplib2.Http()
http = build_http()
resp, content = http.request(
FLAGS.directory_uri,
headers={'X-User-IP': '0.0.0.0'})
Expand Down
9 changes: 5 additions & 4 deletions googleapiclient/_auth.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,6 @@

"""Helpers for authentication using oauth2client or google-auth."""

import httplib2

try:
import google.auth
import google.auth.credentials
Expand All @@ -31,6 +29,8 @@
except ImportError: # pragma: NO COVER
HAS_OAUTH2CLIENT = False

from googleapiclient.http import build_http


def default_credentials():
"""Returns Application Default Credentials."""
Expand Down Expand Up @@ -86,6 +86,7 @@ def authorized_http(credentials):
"""
if HAS_GOOGLE_AUTH and isinstance(
credentials, google.auth.credentials.Credentials):
return google_auth_httplib2.AuthorizedHttp(credentials)
return google_auth_httplib2.AuthorizedHttp(credentials,
http=build_http())
else:
return credentials.authorize(httplib2.Http())
return credentials.authorize(build_http())
9 changes: 7 additions & 2 deletions googleapiclient/discovery.py
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@
from googleapiclient.errors import UnacceptableMimeTypeError
from googleapiclient.errors import UnknownApiNameOrVersion
from googleapiclient.errors import UnknownFileType
from googleapiclient.http import build_http
from googleapiclient.http import BatchHttpRequest
from googleapiclient.http import HttpMock
from googleapiclient.http import HttpMockSequence
Expand Down Expand Up @@ -97,6 +98,7 @@
'version={apiVersion}')
DEFAULT_METHOD_DOC = 'A description of how to use this function'
HTTP_PAYLOAD_METHODS = frozenset(['PUT', 'POST', 'PATCH'])

_MEDIA_SIZE_BIT_SHIFTS = {'KB': 10, 'MB': 20, 'GB': 30, 'TB': 40}
BODY_PARAMETER_DEFAULT_VALUE = {
'description': 'The request body.',
Expand Down Expand Up @@ -213,7 +215,10 @@ def build(serviceName,
'apiVersion': version
}

discovery_http = http if http is not None else httplib2.Http()
if http is None:
discovery_http = build_http()
else:
discovery_http = http

for discovery_url in (discoveryServiceUrl, V2_DISCOVERY_URI,):
requested_url = uritemplate.expand(discovery_url, params)
Expand Down Expand Up @@ -366,7 +371,7 @@ def build_from_document(
# If the service doesn't require scopes then there is no need for
# authentication.
else:
http = httplib2.Http()
http = build_http()

if model is None:
features = service.get('features', [])
Expand Down
20 changes: 20 additions & 0 deletions googleapiclient/http.py
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,8 @@

_TOO_MANY_REQUESTS = 429

DEFAULT_HTTP_TIMEOUT_SEC = 60


def _should_retry_response(resp_status, content):
"""Determines whether a response should be retried.
Expand Down Expand Up @@ -1732,3 +1734,21 @@ def new_request(uri, method='GET', body=None, headers=None,

http.request = new_request
return http


def build_http():
"""Builds httplib2.Http object
Returns:
A httplib2.Http object, which is used to make http requests, and which has timeout set by default.
To override default timeout call
socket.setdefaulttimeout(timeout_in_sec)
before interacting with this method.
"""
if socket.getdefaulttimeout() is not None:
http_timeout = socket.getdefaulttimeout()
else:
http_timeout = DEFAULT_HTTP_TIMEOUT_SEC
return httplib2.Http(timeout=http_timeout)
4 changes: 2 additions & 2 deletions googleapiclient/sample_tools.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,10 +23,10 @@


import argparse
import httplib2
import os

from googleapiclient import discovery
from googleapiclient.http import build_http
from oauth2client import client
from oauth2client import file
from oauth2client import tools
Expand Down Expand Up @@ -88,7 +88,7 @@ def init(argv, name, version, doc, filename, scope=None, parents=[], discovery_f
credentials = storage.get()
if credentials is None or credentials.invalid:
credentials = tools.run_flow(flow, storage, flags)
http = credentials.authorize(http = httplib2.Http())
http = credentials.authorize(http=build_http())

if discovery_filename is None:
# Construct a service object via the discovery service.
Expand Down
20 changes: 13 additions & 7 deletions tests/test__auth.py
Original file line number Diff line number Diff line change
Expand Up @@ -66,10 +66,13 @@ class CredentialsWithScopes(
def test_authorized_http(self):
credentials = mock.Mock(spec=google.auth.credentials.Credentials)

http = _auth.authorized_http(credentials)
authorized_http = _auth.authorized_http(credentials)

self.assertIsInstance(http, google_auth_httplib2.AuthorizedHttp)
self.assertEqual(http.credentials, credentials)
self.assertIsInstance(authorized_http, google_auth_httplib2.AuthorizedHttp)
self.assertEqual(authorized_http.credentials, credentials)
self.assertIsInstance(authorized_http.http, httplib2.Http)
self.assertIsInstance(authorized_http.http.timeout, int)
self.assertGreater(authorized_http.http.timeout, 0)


class TestAuthWithOAuth2Client(unittest2.TestCase):
Expand Down Expand Up @@ -112,11 +115,14 @@ def test_with_scopes_scoped(self):
def test_authorized_http(self):
credentials = mock.Mock(spec=oauth2client.client.Credentials)

http = _auth.authorized_http(credentials)
authorized_http = _auth.authorized_http(credentials)

self.assertEqual(http, credentials.authorize.return_value)
self.assertIsInstance(
credentials.authorize.call_args[0][0], httplib2.Http)
http = credentials.authorize.call_args[0][0]

self.assertEqual(authorized_http, credentials.authorize.return_value)
self.assertIsInstance(http, httplib2.Http)
self.assertIsInstance(http.timeout, int)
self.assertGreater(http.timeout, 0)


class TestAuthWithoutAuth(unittest2.TestCase):
Expand Down
28 changes: 24 additions & 4 deletions tests/test_discovery.py
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@
from googleapiclient.errors import UnacceptableMimeTypeError
from googleapiclient.errors import UnknownApiNameOrVersion
from googleapiclient.errors import UnknownFileType
from googleapiclient.http import build_http
from googleapiclient.http import BatchHttpRequest
from googleapiclient.http import HttpMock
from googleapiclient.http import HttpMockSequence
Expand Down Expand Up @@ -402,13 +403,32 @@ def test_building_with_base_remembers_base(self):
discovery, base=base, credentials=self.MOCK_CREDENTIALS)
self.assertEquals("https://www.googleapis.com/plus/v1/", plus._baseUrl)

def test_building_with_optional_http(self):
def test_building_with_optional_http_with_authorization(self):
discovery = open(datafile('plus.json')).read()
plus = build_from_document(
discovery, base="https://www.googleapis.com/",
credentials=self.MOCK_CREDENTIALS)
self.assertIsInstance(
plus._http, (httplib2.Http, google_auth_httplib2.AuthorizedHttp))

# plus service requires Authorization, hence we expect to see AuthorizedHttp object here
self.assertIsInstance(plus._http, google_auth_httplib2.AuthorizedHttp)
self.assertIsInstance(plus._http.http, httplib2.Http)
self.assertIsInstance(plus._http.http.timeout, int)
self.assertGreater(plus._http.http.timeout, 0)

def test_building_with_optional_http_with_no_authorization(self):
discovery = open(datafile('plus.json')).read()
# Cleanup auth field, so we would use plain http client
discovery = json.loads(discovery)
discovery['auth'] = {}
discovery = json.dumps(discovery)

plus = build_from_document(
discovery, base="https://www.googleapis.com/",
credentials=self.MOCK_CREDENTIALS)
# plus service requires Authorization
self.assertIsInstance(plus._http, httplib2.Http)
self.assertIsInstance(plus._http.timeout, int)
self.assertGreater(plus._http.timeout, 0)

def test_building_with_explicit_http(self):
http = HttpMock()
Expand Down Expand Up @@ -1316,7 +1336,7 @@ def _dummy_zoo_request(self):
zoo_uri = util._add_query_parameter(zoo_uri, 'userIp',
os.environ['REMOTE_ADDR'])

http = httplib2.Http()
http = build_http()
original_request = http.request
def wrapped_request(uri, method='GET', *args, **kwargs):
if uri == zoo_uri:
Expand Down
31 changes: 30 additions & 1 deletion tests/test_http.py
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@
from googleapiclient.errors import BatchError
from googleapiclient.errors import HttpError
from googleapiclient.errors import InvalidChunkSizeError
from googleapiclient.http import build_http
from googleapiclient.http import BatchHttpRequest
from googleapiclient.http import HttpMock
from googleapiclient.http import HttpMockSequence
Expand Down Expand Up @@ -235,7 +236,7 @@ def test_http_request_to_from_json(self):
def _postproc(*kwargs):
pass

http = httplib2.Http()
http = build_http()
media_upload = MediaFileUpload(
datafile('small.png'), chunksize=500, resumable=True)
req = HttpRequest(
Expand Down Expand Up @@ -1341,6 +1342,34 @@ def test_error_response(self):
self.assertRaises(HttpError, request.execute)


class TestHttpBuild(unittest.TestCase):
original_socket_default_timeout = None

@classmethod
def setUpClass(cls):
cls.original_socket_default_timeout = socket.getdefaulttimeout()

@classmethod
def tearDownClass(cls):
socket.setdefaulttimeout(cls.original_socket_default_timeout)

def test_build_http_sets_default_timeout_if_none_specified(self):
socket.setdefaulttimeout(None)
http = build_http()
self.assertIsInstance(http.timeout, int)
self.assertGreater(http.timeout, 0)

def test_build_http_default_timeout_can_be_overridden(self):
socket.setdefaulttimeout(1.5)
http = build_http()
self.assertAlmostEqual(http.timeout, 1.5, delta=0.001)

def test_build_http_default_timeout_can_be_set_to_zero(self):
socket.setdefaulttimeout(0)
http = build_http()
self.assertEquals(http.timeout, 0)


if __name__ == '__main__':
logging.getLogger().setLevel(logging.ERROR)
unittest.main()

0 comments on commit 2243529

Please sign in to comment.