Issue 454 always use finite timeouts with Connection requests

CHANGELOG.md

@@ -11,6 +11,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ### Changed
 
+- `Connection` based requests: always use finite timeouts by default (20 minutes in general, 30 minutes for synchronous execute requests)
+  ([#454](https://github.com/Open-EO/openeo-python-client/issues/454))
+
 ### Removed
 
 ### Fixed

openeo/rest/connection.py

@@ -51,6 +51,11 @@
 
 _log = logging.getLogger(__name__)
 
+# Default timeouts for requests
+# TODO: get default_timeout from config?
+DEFAULT_TIMEOUT = 20 * 60
+DEFAULT_TIMEOUT_SYNCHRONOUS_EXECUTE = 30 * 60
+
 
 class RestApiConnection:
     """Base connection class implementing generic REST API request functionality"""
@@ -66,7 +71,7 @@ def __init__(
         self._root_url = root_url
         self.auth = auth or NullAuth()
         self.session = session or requests.Session()
-        self.default_timeout = default_timeout
+        self.default_timeout = default_timeout or DEFAULT_TIMEOUT
         self.default_headers = {
             "User-Agent": "openeo-python-client/{cv} {py}/{pv} {pl}".format(
                 cv=openeo.client_version(),
@@ -1388,7 +1393,7 @@ def download(
         self,
         graph: Union[dict, FlatGraphableMixin, str, Path],
         outputfile: Union[Path, str, None] = None,
-        timeout: int = 30 * 60,
+        timeout: Optional[int] = None,
     ) -> Union[None, bytes]:
         """
         Downloads the result of a process graph synchronously,
@@ -1401,7 +1406,13 @@ def download(
         :param timeout: timeout to wait for response
         """
         request = self._build_request_with_process_graph(process_graph=graph)
-        response = self.post(path="/result", json=request, expected_status=200, stream=True, timeout=timeout)
+        response = self.post(
+            path="/result",
+            json=request,
+            expected_status=200,
+            stream=True,
+            timeout=timeout or DEFAULT_TIMEOUT_SYNCHRONOUS_EXECUTE,
+        )
 
         if outputfile is not None:
             with Path(outputfile).open(mode="wb") as f:
@@ -1410,7 +1421,11 @@ def download(
         else:
             return response.content
 
-    def execute(self, process_graph: Union[dict, str, Path]):
+    def execute(
+        self,
+        process_graph: Union[dict, str, Path],
+        timeout: Optional[int] = None,
+    ):
         """
         Execute a process graph synchronously and return the result (assumed to be JSON).
 
@@ -1419,7 +1434,12 @@ def execute(self, process_graph: Union[dict, str, Path]):
         :return: parsed JSON response
         """
         req = self._build_request_with_process_graph(process_graph=process_graph)
-        return self.post(path="/result", json=req, expected_status=200).json()
+        return self.post(
+            path="/result",
+            json=req,
+            expected_status=200,
+            timeout=timeout or DEFAULT_TIMEOUT_SYNCHRONOUS_EXECUTE,
+        ).json()
 
     def create_job(
         self,

tests/rest/test_connection.py

@@ -21,7 +21,14 @@
 from openeo.rest.auth.auth import BearerAuth, NullAuth
 from openeo.rest.auth.oidc import OidcException
 from openeo.rest.auth.testing import ABSENT, OidcMock
-from openeo.rest.connection import Connection, RestApiConnection, connect, paginate
+from openeo.rest.connection import (
+    Connection,
+    RestApiConnection,
+    connect,
+    paginate,
+    DEFAULT_TIMEOUT,
+    DEFAULT_TIMEOUT_SYNCHRONOUS_EXECUTE,
+)
 from openeo.util import ContextTimer
 
 from .. import load_json_resource
@@ -266,7 +273,7 @@ def test_connection_with_session():
     conn = Connection("https://oeo.test/", session=session)
     assert conn.capabilities().capabilities["foo"] == "bar"
     session.request.assert_any_call(
-        url="https://oeo.test/", method="get", headers=mock.ANY, stream=mock.ANY, auth=mock.ANY, timeout=None
+        url="https://oeo.test/", method="get", headers=mock.ANY, stream=mock.ANY, auth=mock.ANY, timeout=DEFAULT_TIMEOUT
     )
 
 
@@ -278,7 +285,7 @@ def test_connect_with_session():
     conn = connect("https://oeo.test/", session=session)
     assert conn.capabilities().capabilities["foo"] == "bar"
     session.request.assert_any_call(
-        url="https://oeo.test/", method="get", headers=mock.ANY, stream=mock.ANY, auth=mock.ANY, timeout=None
+        url="https://oeo.test/", method="get", headers=mock.ANY, stream=mock.ANY, auth=mock.ANY, timeout=DEFAULT_TIMEOUT
     )
 
 
@@ -2515,7 +2522,7 @@ def test_default_timeout_default(requests_mock):
     requests_mock.get(API_URL, json={"api_version": "1.0.0"})
     requests_mock.get("/foo", text=lambda req, ctx: repr(req.timeout))
     conn = connect(API_URL)
-    assert conn.get("/foo").text == 'None'
+    assert conn.get("/foo").text == str(DEFAULT_TIMEOUT)
     assert conn.get("/foo", timeout=5).text == '5'
 
 
@@ -2532,6 +2539,32 @@ def flat_graph(self) -> typing.Dict[str, dict]:
         return {"foo1": {"process_id": "foo"}}
 
 
+@pytest.mark.parametrize(
+    "pg",
+    [
+        {"foo1": {"process_id": "foo"}},
+        {"process_graph": {"foo1": {"process_id": "foo"}}},
+        DummyFlatGraphable(),
+    ],
+)
+def test_download_100(requests_mock, pg):
+    requests_mock.get(API_URL, json={"api_version": "1.0.0"})
+    conn = Connection(API_URL)
+    with mock.patch.object(conn, "request") as request:
+        conn.download(pg)
+    assert request.call_args_list == [
+        mock.call(
+            "post",
+            path="/result",
+            allow_redirects=False,
+            stream=True,
+            expected_status=200,
+            json={"process": {"process_graph": {"foo1": {"process_id": "foo"}}}},
+            timeout=DEFAULT_TIMEOUT_SYNCHRONOUS_EXECUTE,
+        )
+    ]
+
+
 @pytest.mark.parametrize(
     "pg",
     [
@@ -2547,8 +2580,12 @@ def test_execute_100(requests_mock, pg):
         conn.execute(pg)
     assert request.call_args_list == [
         mock.call(
-            "post", path="/result", allow_redirects=False, expected_status=200,
-            json={"process": {"process_graph": {"foo1": {"process_id": "foo"}}}}
+            "post",
+            path="/result",
+            allow_redirects=False,
+            expected_status=200,
+            json={"process": {"process_graph": {"foo1": {"process_id": "foo"}}}},
+            timeout=DEFAULT_TIMEOUT_SYNCHRONOUS_EXECUTE,
         )
     ]