Skip to content

Commit

Permalink
feat(core): change default api_request() timeout to non-None (#10219)
Browse files Browse the repository at this point in the history
  • Loading branch information
plamut authored Jan 29, 2020
1 parent 8a0e129 commit 9ad21f4
Show file tree
Hide file tree
Showing 2 changed files with 34 additions and 14 deletions.
14 changes: 8 additions & 6 deletions core/google/cloud/_http.py
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,8 @@
'extra_headers' instead.
"""

_DEFAULT_TIMEOUT = 60 # in seconds


class Connection(object):
"""A generic connection to Google Cloud Platform.
Expand Down Expand Up @@ -222,7 +224,7 @@ def _make_request(
content_type=None,
headers=None,
target_object=None,
timeout=None,
timeout=_DEFAULT_TIMEOUT,
):
"""A low level method to send a request to the API.
Expand Down Expand Up @@ -253,7 +255,7 @@ def _make_request(
:type timeout: float or tuple
:param timeout: (optional) The amount of time, in seconds, to wait
for the server response. By default, the method waits indefinitely.
for the server response.
Can also be passed as a tuple (connect_timeout, read_timeout).
See :meth:`requests.Session.request` documentation for details.
Expand All @@ -276,7 +278,7 @@ def _make_request(
)

def _do_request(
self, method, url, headers, data, target_object, timeout=None
self, method, url, headers, data, target_object, timeout=_DEFAULT_TIMEOUT
): # pylint: disable=unused-argument
"""Low-level helper: perform the actual API request over HTTP.
Expand All @@ -301,7 +303,7 @@ def _do_request(
:type timeout: float or tuple
:param timeout: (optional) The amount of time, in seconds, to wait
for the server response. By default, the method waits indefinitely.
for the server response.
Can also be passed as a tuple (connect_timeout, read_timeout).
See :meth:`requests.Session.request` documentation for details.
Expand All @@ -325,7 +327,7 @@ def api_request(
api_version=None,
expect_json=True,
_target_object=None,
timeout=None,
timeout=_DEFAULT_TIMEOUT,
):
"""Make a request over the HTTP transport to the API.
Expand Down Expand Up @@ -382,7 +384,7 @@ def api_request(
:type timeout: float or tuple
:param timeout: (optional) The amount of time, in seconds, to wait
for the server response. By default, the method waits indefinitely.
for the server response.
Can also be passed as a tuple (connect_timeout, read_timeout).
See :meth:`requests.Session.request` documentation for details.
Expand Down
34 changes: 26 additions & 8 deletions core/tests/unit/test__http.py
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,12 @@ class TestJSONConnection(unittest.TestCase):
JSON_HEADERS = {"content-type": "application/json"}
EMPTY_JSON_RESPONSE = make_response(content=b"{}", headers=JSON_HEADERS)

@staticmethod
def _get_default_timeout():
from google.cloud._http import _DEFAULT_TIMEOUT

return _DEFAULT_TIMEOUT

@staticmethod
def _get_target_class():
from google.cloud._http import JSONConnection
Expand Down Expand Up @@ -217,7 +223,11 @@ def test__make_request_no_data_no_content_type_no_headers(self):
CLIENT_INFO_HEADER: conn.user_agent,
}
http.request.assert_called_once_with(
method="GET", url=url, headers=expected_headers, data=None, timeout=None
method="GET",
url=url,
headers=expected_headers,
data=None,
timeout=self._get_default_timeout(),
)

def test__make_request_w_data_no_extra_headers(self):
Expand All @@ -238,7 +248,11 @@ def test__make_request_w_data_no_extra_headers(self):
CLIENT_INFO_HEADER: conn.user_agent,
}
http.request.assert_called_once_with(
method="GET", url=url, headers=expected_headers, data=data, timeout=None
method="GET",
url=url,
headers=expected_headers,
data=data,
timeout=self._get_default_timeout(),
)

def test__make_request_w_extra_headers(self):
Expand All @@ -258,7 +272,11 @@ def test__make_request_w_extra_headers(self):
CLIENT_INFO_HEADER: conn.user_agent,
}
http.request.assert_called_once_with(
method="GET", url=url, headers=expected_headers, data=None, timeout=None
method="GET",
url=url,
headers=expected_headers,
data=None,
timeout=self._get_default_timeout(),
)

def test__make_request_w_timeout(self):
Expand Down Expand Up @@ -309,7 +327,7 @@ def test_api_request_defaults(self):
url=expected_url,
headers=expected_headers,
data=None,
timeout=None,
timeout=self._get_default_timeout(),
)

def test_api_request_w_non_json_response(self):
Expand Down Expand Up @@ -352,7 +370,7 @@ def test_api_request_w_query_params(self):
url=mock.ANY,
headers=expected_headers,
data=None,
timeout=None,
timeout=self._get_default_timeout(),
)

url = http.request.call_args[1]["url"]
Expand Down Expand Up @@ -386,7 +404,7 @@ def test_api_request_w_headers(self):
url=mock.ANY,
headers=expected_headers,
data=None,
timeout=None,
timeout=self._get_default_timeout(),
)

def test_api_request_w_extra_headers(self):
Expand Down Expand Up @@ -416,7 +434,7 @@ def test_api_request_w_extra_headers(self):
url=mock.ANY,
headers=expected_headers,
data=None,
timeout=None,
timeout=self._get_default_timeout(),
)

def test_api_request_w_data(self):
Expand All @@ -443,7 +461,7 @@ def test_api_request_w_data(self):
url=mock.ANY,
headers=expected_headers,
data=expected_data,
timeout=None,
timeout=self._get_default_timeout(),
)

def test_api_request_w_timeout(self):
Expand Down

0 comments on commit 9ad21f4

Please sign in to comment.