Add a default timeout for the http_request method

docs/latest/.buildinfo

@@ -1,4 +1,4 @@
 # Sphinx build info version 1
 # This file hashes the configuration used when building these files. When it is not found, a full rebuild will be done.
-config: 7e1b338c2c9c8a7781dd9b0067280fb3
+config: 883e2c6bf1289e72ace5b1e3a389b1fe
 tags: 645f666f9bcd5a90fca523b33c5a78b7

google/resumable_media/requests/_helpers.py

@@ -25,6 +25,12 @@
 
 
 _DEFAULT_RETRY_STRATEGY = common.RetryStrategy()
+# The number of seconds to wait to establish a connection
+# (connect() call on socket). Avoid setting this to a multiple of 3 to not
+# Align with TCP Retransmission timing. (typically 2.5-3s)
+_DEFAULT_CONNECT_TIMEOUT = 61
+# The number of seconds to wait between bytes sent from the server.
+_DEFAULT_READ_TIMEOUT = 60
 
 
 class RequestsMixin(object):
@@ -94,6 +100,10 @@ def http_request(transport, method, url, data=None, headers=None,
     Returns:
         ~requests.Response: The return value of ``transport.request()``.
     """
+    if "timeout" not in transport_kwargs:
+        transport_kwargs["timeout"] = (
+            _DEFAULT_CONNECT_TIMEOUT, _DEFAULT_READ_TIMEOUT)
+
     func = functools.partial(
         transport.request, method, url, data=data, headers=headers,
         **transport_kwargs)

tests/unit/requests/test__helpers.py

@@ -17,6 +17,8 @@
 
 from google.resumable_media.requests import _helpers
 
+EXPECTED_TIMEOUT = (61, 60)
+
 
 class TestRequestsMixin(object):
 
@@ -42,14 +44,27 @@ def test_http_request():
     url = u'http://test.invalid'
     data = mock.sentinel.data
     headers = {u'one': u'fish', u'blue': u'fish'}
+    timeout = mock.sentinel.timeout
     ret_val = _helpers.http_request(
         transport, method, url, data=data, headers=headers,
-        extra1=b'work', extra2=125.5)
+        extra1=b'work', extra2=125.5, timeout=timeout)
 
     assert ret_val is responses[0]
     transport.request.assert_called_once_with(
         method, url, data=data, headers=headers,
-        extra1=b'work', extra2=125.5)
+        extra1=b'work', extra2=125.5, timeout=timeout)
+
+
+def test_http_request_defaults():
+    transport, responses = _make_transport(http_client.OK)
+    method = u'POST'
+    url = u'http://test.invalid'
+    ret_val = _helpers.http_request(transport, method, url)
+
+    assert ret_val is responses[0]
+    transport.request.assert_called_once_with(
+        method, url, data=None, headers=None,
+        timeout=EXPECTED_TIMEOUT)
 
 
 def _make_response(status_code):

tests/unit/requests/test_download.py

@@ -25,6 +25,7 @@
 EXAMPLE_URL = (
     u'https://www.googleapis.com/download/storage/v1/b/'
     u'{BUCKET}/o/{OBJECT}?alt=media')
+EXPECTED_TIMEOUT = (61, 60)
 
 
 class TestDownload(object):
@@ -149,7 +150,7 @@ def _consume_helper(
             assert stream is not None
             called_kwargs[u'stream'] = True
         transport.request.assert_called_once_with(
-            u'GET', EXAMPLE_URL, **called_kwargs)
+            u'GET', EXAMPLE_URL, timeout=EXPECTED_TIMEOUT, **called_kwargs)
 
         range_bytes = u'bytes={:d}-{:d}'.format(0, end)
         assert download._headers[u'range'] == range_bytes
@@ -224,7 +225,8 @@ def test_consume_with_stream_hash_check_fail(self):
 
         # Check mocks.
         transport.request.assert_called_once_with(
-            u'GET', EXAMPLE_URL, data=None, headers={}, stream=True)
+            u'GET', EXAMPLE_URL, data=None, headers={}, stream=True,
+            timeout=EXPECTED_TIMEOUT)
 
     def test_consume_with_headers(self):
         headers = {}  # Empty headers
@@ -302,7 +304,8 @@ def test_consume_next_chunk(self):
         range_bytes = u'bytes={:d}-{:d}'.format(start, start + chunk_size - 1)
         download_headers = {u'range': range_bytes}
         transport.request.assert_called_once_with(
-            u'GET', EXAMPLE_URL, data=None, headers=download_headers)
+            u'GET', EXAMPLE_URL, data=None, headers=download_headers,
+            timeout=EXPECTED_TIMEOUT)
         assert stream.getvalue() == data
         # Go back and check the internal state after consuming the chunk.
         assert not download.finished

tests/unit/requests/test_upload.py

@@ -35,6 +35,7 @@
 BASIC_CONTENT = u'text/plain'
 JSON_TYPE = u'application/json; charset=UTF-8'
 JSON_TYPE_LINE = b'content-type: application/json; charset=UTF-8\r\n'
+EXPECTED_TIMEOUT = (61, 60)
 
 
 class TestSimpleUpload(object):
@@ -51,7 +52,8 @@ def test_transmit(self):
         assert ret_val is transport.request.return_value
         upload_headers = {u'content-type': content_type}
         transport.request.assert_called_once_with(
-            u'POST', SIMPLE_URL, data=data, headers=upload_headers)
+            u'POST', SIMPLE_URL, data=data, headers=upload_headers,
+            timeout=EXPECTED_TIMEOUT)
         assert upload.finished
 
 
@@ -84,7 +86,7 @@ def test_transmit(self, mock_get_boundary):
         upload_headers = {u'content-type': multipart_type}
         transport.request.assert_called_once_with(
             u'POST', MULTIPART_URL, data=expected_payload,
-            headers=upload_headers)
+            headers=upload_headers, timeout=EXPECTED_TIMEOUT)
         assert upload.finished
         mock_get_boundary.assert_called_once_with()
 
@@ -121,7 +123,8 @@ def test_initiate(self):
             u'x-upload-content-length': u'{:d}'.format(total_bytes),
         }
         transport.request.assert_called_once_with(
-            u'POST', RESUMABLE_URL, data=json_bytes, headers=expected_headers)
+            u'POST', RESUMABLE_URL, data=json_bytes, headers=expected_headers,
+            timeout=EXPECTED_TIMEOUT)
 
     @staticmethod
     def _upload_in_flight(data, headers=None):
@@ -170,7 +173,7 @@ def test_transmit_next_chunk(self):
         }
         transport.request.assert_called_once_with(
             u'PUT', upload.resumable_url, data=payload,
-            headers=expected_headers)
+            headers=expected_headers, timeout=EXPECTED_TIMEOUT)
 
     def test_recover(self):
         upload = upload_mod.ResumableUpload(RESUMABLE_URL, ONE_MB)
@@ -191,7 +194,8 @@ def test_recover(self):
         upload._stream.seek.assert_called_once_with(end + 1)
         expected_headers = {u'content-range': u'bytes */*'}
         transport.request.assert_called_once_with(
-            u'PUT', upload.resumable_url, data=None, headers=expected_headers)
+            u'PUT', upload.resumable_url, data=None, headers=expected_headers,
+            timeout=EXPECTED_TIMEOUT)
 
 
 def _make_response(status_code=http_client.OK, headers=None):