Merge pull request #445 from rstudio/tdstein/442-Adjust-the-Default-Timeout-for-Long-Lived-Task

Adds CONNECT_TASK_TIMEOUT environment variable.

Author: tdstein

CHANGELOG.md

@@ -4,6 +4,12 @@ All notable changes to this project will be documented in this file.
 The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 
+## Unreleased
+
+### Added
+
+- The `CONNECT_TASK_TIMEOUT` environment variable, which configures the timeout for [task based operations](https://docs.posit.co/connect/api/#get-/v1/tasks/-id-). This value translates into seconds (e.g., `CONNECT_TASK_TIMEOUT=60` is equivalent to 60 seconds.) By default, this value is set to [`sys.maxsize`](https://docs.python.org/3/library/sys.html#sys.maxsize).
+
 ## [1.18.0] - 2023-06-27
 
 ### Added

rsconnect/api.py

@@ -28,7 +28,7 @@
 from .metadata import ServerStore, AppStore
 from .exception import RSConnectException
 from .bundle import _default_title, fake_module_file_from_directory
-from .timeouts import get_timeout
+from .timeouts import get_task_timeout
 
 
 class AbstractRemoteServer:
@@ -292,21 +292,27 @@ def get_content(self, content_guid):
         return results
 
     def wait_for_task(
-        self, task_id, log_callback, abort_func=lambda: False, timeout=get_timeout(), poll_wait=0.5, raise_on_error=True
+        self,
+        task_id,
+        log_callback,
+        abort_func=lambda: False,
+        timeout=get_task_timeout(),
+        poll_wait=0.5,
+        raise_on_error=True,
     ):
-        last_status = None
-        ending = time.time() + timeout if timeout else 999999999999
 
         if log_callback is None:
             log_lines = []
             log_callback = log_lines.append
         else:
             log_lines = None
 
+        last_status = None
+        start_time = time.time()
         sleep_duration = 0.5
         time_slept = 0
         while True:
-            if time.time() >= ending:
+            if (time.time() - start_time) > timeout:
                 raise RSConnectException("Task timed out after %d seconds" % timeout)
             elif abort_func():
                 raise RSConnectException("Task aborted.")
@@ -741,7 +747,7 @@ def emit_task_log(
         task_id: int = None,
         log_callback=connect_logger,
         abort_func: Callable[[], bool] = lambda: False,
-        timeout: int = None,
+        timeout: int = get_task_timeout(),
         poll_wait: float = 0.5,
         raise_on_error: bool = True,
     ):
@@ -1171,7 +1177,7 @@ def get_current_user(self):
         self._server.handle_bad_response(response)
         return response
 
-    def wait_until_task_is_successful(self, task_id, timeout=get_timeout()):
+    def wait_until_task_is_successful(self, task_id, timeout=get_task_timeout()):
         print()
         print("Waiting for task: {}".format(task_id))
         start_time = time.time()
@@ -1448,7 +1454,7 @@ def emit_task_log(
     task_id,
     log_callback,
     abort_func=lambda: False,
-    timeout=get_timeout(),
+    timeout=get_task_timeout(),
     poll_wait=0.5,
     raise_on_error=True,
 ):

rsconnect/http_support.py

@@ -13,7 +13,7 @@
 from six.moves.urllib_parse import urlparse, urlencode, urljoin
 import base64
 
-from .timeouts import get_timeout
+from .timeouts import get_request_timeout
 
 _user_agent = "rsconnect-python/%s" % VERSION
 
@@ -31,7 +31,7 @@ def _create_plain_connection(host_name, port, disable_tls_check, ca_data):
     :param ca_data: any certificate authority information to use (ignored).
     :return: a plain HTTP connection.
     """
-    timeout = get_timeout()
+    timeout = get_request_timeout()
     logger.debug(f"The HTTPConnection timeout is set to '{timeout}' seconds")
     return http.HTTPConnection(host_name, port=(port or http.HTTP_PORT), timeout=timeout)
 
@@ -77,7 +77,7 @@ def _create_ssl_connection(host_name, port, disable_tls_check, ca_data):
         raise ValueError("Cannot both disable TLS checking and provide a custom certificate")
     _, _, proxyHost, proxyPort = _get_proxy()
     headers = _get_proxy_headers()
-    timeout = get_timeout()
+    timeout = get_request_timeout()
     logger.debug(f"The HTTPSConnection timeout is set to '{timeout}' seconds")
     if ca_data is not None:
         return http.HTTPSConnection(

rsconnect/timeouts.py

@@ -1,13 +1,17 @@
 import os
+import sys
 from typing import Union
 
 from rsconnect.exception import RSConnectException
 
-_CONNECT_REQUEST_TIMEOUT_KEY = "CONNECT_REQUEST_TIMEOUT"
-_CONNECT_REQUEST_TIMEOUT_DEFAULT_VALUE = "300"
+_CONNECT_REQUEST_TIMEOUT_KEY: str = "CONNECT_REQUEST_TIMEOUT"
+_CONNECT_REQUEST_TIMEOUT_DEFAULT_VALUE: str = "300"
 
+_CONNECT_TASK_TIMEOUT_KEY: str = "CONNECT_TASK_TIMEOUT"
+_CONNECT_TASK_TIMEOUT_DEFAULT_VALUE: str = str(sys.maxsize)
 
-def get_timeout() -> int:
+
+def get_request_timeout() -> int:
     """Gets the timeout from the CONNECT_REQUEST_TIMEOUT env variable.
 
     The timeout value is intended to be interpreted in seconds. A value of 60 is equal to sixty seconds, or one minute.
@@ -30,12 +34,40 @@ def get_timeout() -> int:
         timeout = int(timeout)
     except ValueError:
         raise RSConnectException(
-            f"'CONNECT_REQUEST_TIMEOUT' is set to '{timeout}'. The value must be a natural number."
+            f"'CONNECT_REQUEST_TIMEOUT' is set to '{timeout}'. The value must be a non-negative integer."
         )
 
     if timeout < 0:
         raise RSConnectException(
-            f"'CONNECT_REQUEST_TIMEOUT' is set to '{timeout}'. The value must be a natural number."
+            f"'CONNECT_REQUEST_TIMEOUT' is set to '{timeout}'. The value must be a non-negative integer."
         )
 
     return timeout
+
+
+def get_task_timeout() -> int:
+    """Gets the timeout from the CONNECT_TASK_TIMEOUT env variable.
+
+    The timeout value is intended to be interpreted in seconds. A value of 60 is equal to sixty seconds, or one minute.
+
+    If CONNECT_TASK_TIMEOUT is unset, a default value of sys.maxsize is used.
+
+    If CONNECT_TASK_TIMEOUT is set to a value less or equal to 0, an `RSConnectException` is raised.
+
+    The primary intent for this method is for usage with the `api` module. Specifically, for setting the timeout
+    parameter in the method `wait_for_task`.
+
+    :raises: `RSConnectException` if CONNECT_TASK_TIMEOUT is not a positive integer.
+    :return: the timeout value
+    """
+    timeout: Union[int, str] = os.environ.get(_CONNECT_TASK_TIMEOUT_KEY, _CONNECT_TASK_TIMEOUT_DEFAULT_VALUE)
+
+    try:
+        timeout = int(timeout)
+    except ValueError:
+        raise RSConnectException(f"'CONNECT_TASK_TIMEOUT' is set to '{timeout}'. The value must be a positive integer.")
+
+    if timeout <= 0:
+        raise RSConnectException(f"'CONNECT_TASK_TIMEOUT' is set to '{timeout}'. The value must be a positive integer.")
+
+    return timeout

tests/test_timeouts.py

@@ -1,33 +1,59 @@
 import os
+import sys
 
 from unittest import TestCase
 from unittest.mock import patch
 
 from rsconnect.exception import RSConnectException
-from rsconnect.timeouts import get_timeout
+from rsconnect.timeouts import get_request_timeout, get_task_timeout
 
 
-class GetTimeoutTestCase(TestCase):
+class GetRequestTimeoutTestCase(TestCase):
     def test_get_default_timeout(self):
-        timeout = get_timeout()
+        timeout = get_request_timeout()
         self.assertEqual(300, timeout)
 
     def test_get_valid_timeout_from_environment(self):
         with patch.dict(os.environ, {"CONNECT_REQUEST_TIMEOUT": "24"}):
-            timeout = get_timeout()
+            timeout = get_request_timeout()
             self.assertEqual(24, timeout)
 
     def test_get_zero_timeout_from_environment(self):
         with patch.dict(os.environ, {"CONNECT_REQUEST_TIMEOUT": "0"}):
-            timeout = get_timeout()
+            timeout = get_request_timeout()
             self.assertEqual(0, timeout)
 
     def test_get_invalid_timeout_from_environment(self):
         with patch.dict(os.environ, {"CONNECT_REQUEST_TIMEOUT": "foobar"}):
             with self.assertRaises(RSConnectException):
-                get_timeout()
+                get_request_timeout()
 
     def test_get_negative_timeout_from_environment(self):
         with patch.dict(os.environ, {"CONNECT_REQUEST_TIMEOUT": "-24"}):
             with self.assertRaises(RSConnectException):
-                get_timeout()
+                get_request_timeout()
+
+class GetTaskTimeoutTestCase(TestCase):
+    def test_get_default_timeout(self):
+        timeout = get_task_timeout()
+        self.assertEqual(sys.maxsize, timeout)
+
+    def test_get_valid_timeout_from_environment(self):
+        with patch.dict(os.environ, {"CONNECT_TASK_TIMEOUT": "24"}):
+            timeout = get_task_timeout()
+            self.assertEqual(24, timeout)
+
+    def test_get_zero_timeout_from_environment(self):
+        with patch.dict(os.environ, {"CONNECT_TASK_TIMEOUT": "0"}):
+            with self.assertRaises(RSConnectException):
+                get_task_timeout()
+
+    def test_get_invalid_timeout_from_environment(self):
+        with patch.dict(os.environ, {"CONNECT_TASK_TIMEOUT": "foobar"}):
+            with self.assertRaises(RSConnectException):
+                get_task_timeout()
+
+    def test_get_negative_timeout_from_environment(self):
+        with patch.dict(os.environ, {"CONNECT_TASK_TIMEOUT": "-24"}):
+            with self.assertRaises(RSConnectException):
+                get_task_timeout()