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.

Sign up for GitHub

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

Jump to bottom

feat(core): change default api_request() timeout to non-None #10219

Merged
merged 1 commit into from
Jan 29, 2020
Merged

feat(core): change default api_request() timeout to non-None #10219

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
14 changes: 8 additions & 6 deletions core/google/cloud/_http.py
View file
Open in desktop
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
View file
Open in desktop
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