Skip to content

Commit

Permalink
feat(core): add timeout param to JSONConnection.api_request() (#9915)
Browse files Browse the repository at this point in the history
  • Loading branch information
@plamut @tswast
plamut authored and tswast committed Dec 4, 2019
1 parent 38d8f2b commit 7bd36bd
Show file tree
Hide file tree
Showing 2 changed files with 110 additions and 11 deletions.
34 changes: 31 additions & 3 deletions core/google/cloud/_http.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -222,6 +222,7 @@ def _make_request(
content_type=None,
headers=None,
target_object=None,
timeout=None,
):
"""A low level method to send a request to the API.
Expand Down Expand Up @@ -250,6 +251,13 @@ def _make_request(
custom behavior, for example, to defer an HTTP request and complete
initialization of the object at a later time.
: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.
Can also be passed as a tuple (connect_timeout, read_timeout).
See :meth:`requests.Session.request` documentation for details.
:rtype: :class:`requests.Response`
:returns: The HTTP response.
"""
Expand All @@ -263,10 +271,12 @@ def _make_request(
headers[CLIENT_INFO_HEADER] = self.user_agent
headers["User-Agent"] = self.user_agent

return self._do_request(method, url, headers, data, target_object)
return self._do_request(
method, url, headers, data, target_object, timeout=timeout
)

def _do_request(
self, method, url, headers, data, target_object
self, method, url, headers, data, target_object, timeout=None
): # pylint: disable=unused-argument
"""Low-level helper: perform the actual API request over HTTP.
Expand All @@ -289,10 +299,19 @@ def _do_request(
(Optional) Unused ``target_object`` here but may be used by a
superclass.
: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.
Can also be passed as a tuple (connect_timeout, read_timeout).
See :meth:`requests.Session.request` documentation for details.
:rtype: :class:`requests.Response`
:returns: The HTTP response.
"""
return self.http.request(url=url, method=method, headers=headers, data=data)
return self.http.request(
url=url, method=method, headers=headers, data=data, timeout=timeout
)

def api_request(
self,
Expand All @@ -306,6 +325,7 @@ def api_request(
api_version=None,
expect_json=True,
_target_object=None,
timeout=None,
):
"""Make a request over the HTTP transport to the API.
Expand Down Expand Up @@ -360,6 +380,13 @@ def api_request(
can allow custom behavior, for example, to defer an HTTP request
and complete initialization of the object at a later time.
: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.
Can also be passed as a tuple (connect_timeout, read_timeout).
See :meth:`requests.Session.request` documentation for details.
:raises ~google.cloud.exceptions.GoogleCloudError: if the response code
is not 200 OK.
:raises ValueError: if the response content type is not JSON.
Expand Down Expand Up @@ -387,6 +414,7 @@ def api_request(
content_type=content_type,
headers=headers,
target_object=_target_object,
timeout=timeout,
)

if not 200 <= response.status_code < 300:
Expand Down
87 changes: 79 additions & 8 deletions core/tests/unit/test__http.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -217,7 +217,7 @@ 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
method="GET", url=url, headers=expected_headers, data=None, timeout=None
)

def test__make_request_w_data_no_extra_headers(self):
Expand All @@ -238,7 +238,7 @@ 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
method="GET", url=url, headers=expected_headers, data=data, timeout=None
)

def test__make_request_w_extra_headers(self):
Expand All @@ -258,7 +258,30 @@ 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
method="GET", url=url, headers=expected_headers, data=None, timeout=None
)

def test__make_request_w_timeout(self):
from google.cloud._http import CLIENT_INFO_HEADER

http = make_requests_session([make_response()])
client = mock.Mock(_http=http, spec=["_http"])
conn = self._make_one(client)

url = "http://example.com/test"
conn._make_request("GET", url, timeout=(5.5, 2.8))

expected_headers = {
"Accept-Encoding": "gzip",
"User-Agent": conn.user_agent,
CLIENT_INFO_HEADER: conn.user_agent,
}
http.request.assert_called_once_with(
method="GET",
url=url,
headers=expected_headers,
data=None,
timeout=(5.5, 2.8),
)

def test_api_request_defaults(self):
Expand All @@ -282,7 +305,11 @@ def test_api_request_defaults(self):
base=conn.API_BASE_URL, version=conn.API_VERSION, path=path
)
http.request.assert_called_once_with(
method="GET", url=expected_url, headers=expected_headers, data=None
method="GET",
url=expected_url,
headers=expected_headers,
data=None,
timeout=None,
)

def test_api_request_w_non_json_response(self):
Expand Down Expand Up @@ -321,7 +348,11 @@ def test_api_request_w_query_params(self):
CLIENT_INFO_HEADER: conn.user_agent,
}
http.request.assert_called_once_with(
method="GET", url=mock.ANY, headers=expected_headers, data=None
method="GET",
url=mock.ANY,
headers=expected_headers,
data=None,
timeout=None,
)

url = http.request.call_args[1]["url"]
Expand Down Expand Up @@ -351,7 +382,11 @@ def test_api_request_w_headers(self):
CLIENT_INFO_HEADER: conn.user_agent,
}
http.request.assert_called_once_with(
method="GET", url=mock.ANY, headers=expected_headers, data=None
method="GET",
url=mock.ANY,
headers=expected_headers,
data=None,
timeout=None,
)

def test_api_request_w_extra_headers(self):
Expand All @@ -377,7 +412,11 @@ def test_api_request_w_extra_headers(self):
CLIENT_INFO_HEADER: conn.user_agent,
}
http.request.assert_called_once_with(
method="GET", url=mock.ANY, headers=expected_headers, data=None
method="GET",
url=mock.ANY,
headers=expected_headers,
data=None,
timeout=None,
)

def test_api_request_w_data(self):
Expand All @@ -400,7 +439,39 @@ def test_api_request_w_data(self):
}

http.request.assert_called_once_with(
method="POST", url=mock.ANY, headers=expected_headers, data=expected_data
method="POST",
url=mock.ANY,
headers=expected_headers,
data=expected_data,
timeout=None,
)

def test_api_request_w_timeout(self):
from google.cloud._http import CLIENT_INFO_HEADER

http = make_requests_session(
[make_response(content=b"{}", headers=self.JSON_HEADERS)]
)
client = mock.Mock(_http=http, spec=["_http"])
conn = self._make_mock_one(client)
path = "/path/required"

self.assertEqual(conn.api_request("GET", path, timeout=(2.2, 3.3)), {})

expected_headers = {
"Accept-Encoding": "gzip",
"User-Agent": conn.user_agent,
CLIENT_INFO_HEADER: conn.user_agent,
}
expected_url = "{base}/mock/{version}{path}".format(
base=conn.API_BASE_URL, version=conn.API_VERSION, path=path
)
http.request.assert_called_once_with(
method="GET",
url=expected_url,
headers=expected_headers,
data=None,
timeout=(2.2, 3.3),
)

def test_api_request_w_404(self):
Expand Down

0 comments on commit 7bd36bd

Please sign in to comment.