From fcaee45de820d2e9e81b8502cd99e6c31c7aace1 Mon Sep 17 00:00:00 2001 From: Peter Lamut Date: Tue, 28 Jan 2020 19:02:46 +0000 Subject: [PATCH] feat(core): change default api_request() timeout to non-None --- core/google/cloud/_http.py | 14 ++++++++------ core/tests/unit/test__http.py | 34 ++++++++++++++++++++++++++-------- 2 files changed, 34 insertions(+), 14 deletions(-) diff --git a/core/google/cloud/_http.py b/core/google/cloud/_http.py index 8ffbc77a4b75..12315c247cdb 100644 --- a/core/google/cloud/_http.py +++ b/core/google/cloud/_http.py @@ -46,6 +46,8 @@ 'extra_headers' instead. """ +_DEFAULT_TIMEOUT = 60 # in seconds + class Connection(object): """A generic connection to Google Cloud Platform. @@ -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. @@ -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. @@ -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. @@ -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. @@ -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. @@ -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. diff --git a/core/tests/unit/test__http.py b/core/tests/unit/test__http.py index 618f02930631..145e30295e65 100644 --- a/core/tests/unit/test__http.py +++ b/core/tests/unit/test__http.py @@ -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 @@ -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): @@ -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): @@ -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): @@ -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): @@ -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"] @@ -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): @@ -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): @@ -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):