Merge pull request #349 from rstudio/346-add-timeout-configuration

Adds CONNECT_REQUEST_TIMEOUT env variable

Author: tdstein

CHANGELOG.md

@@ -4,6 +4,11 @@ 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_REQUEST_TIMEOUT` environment variable, which configures the request timeout for all blocking HTTP and HTTPS operations. This value translates into seconds (e.g., `CONNECT_REQUEST_TIMEOUT=60` is equivalent to 60 seconds.) By default, this value is 300. 
+
 ## [1.15.0] - 2023-03-15
 
 ### Added
@@ -13,10 +18,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 ### Changed
 - `deploy html` was refactored. Its behavior is described below.
 
-### deploy html
-- specifying a directory in the path will result in that entire directory*, subdirectories, and sub contents included in the deploy bundle
-    - the entire directory is included whether or not an entrypoint was supplied
-
+#### Deploying HTML
+Specifying a directory in the path will result in that entire directory*, subdirectories, and sub contents included in the deploy bundle. The entire directory is included whether or not an entrypoint was supplied
 
 
 e.g.

rsconnect/actions_content.py

@@ -35,7 +35,7 @@ def build_add_content(connect_server, content_guids_with_bundle):
             + "please wait for it to finish before adding new content."
         )
 
-    with RSConnectClient(connect_server, timeout=120) as client:
+    with RSConnectClient(connect_server) as client:
         if len(content_guids_with_bundle) == 1:
             all_content = [client.content_get(content_guids_with_bundle[0].guid)]
         else:
@@ -290,7 +290,7 @@ def download_bundle(connect_server, guid_with_bundle):
     """
     :param guid_with_bundle: models.ContentGuidWithBundle
     """
-    with RSConnectClient(connect_server, timeout=120) as client:
+    with RSConnectClient(connect_server) as client:
         # bundle_id not provided so grab the latest
         if not guid_with_bundle.bundle_id:
             content = client.get_content(guid_with_bundle.guid)
@@ -309,7 +309,7 @@ def get_content(connect_server, guid):
     :param guid: a single guid as a string or list of guids.
     :return: a list of content items.
     """
-    with RSConnectClient(connect_server, timeout=120) as client:
+    with RSConnectClient(connect_server) as client:
         if isinstance(guid, str):
             result = [client.get_content(guid)]
         else:
@@ -320,7 +320,7 @@ def get_content(connect_server, guid):
 def search_content(
     connect_server, published, unpublished, content_type, r_version, py_version, title_contains, order_by
 ):
-    with RSConnectClient(connect_server, timeout=120) as client:
+    with RSConnectClient(connect_server) as client:
         result = client.search_content()
         result = _apply_content_filters(
             result, published, unpublished, content_type, r_version, py_version, title_contains

rsconnect/api.py

@@ -28,6 +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
 
 
 class AbstractRemoteServer:
@@ -127,15 +128,14 @@ def __init__(self, url: str):
 
 
 class RSConnectClient(HTTPServer):
-    def __init__(self, server: RSConnectServer, cookies=None, timeout=30):
+    def __init__(self, server: RSConnectServer, cookies=None):
         if cookies is None:
             cookies = server.cookie_jar
         super().__init__(
             append_to_path(server.url, "__api__"),
             server.insecure,
             server.ca_data,
             cookies,
-            timeout,
         )
         self._server = server
 
@@ -282,7 +282,7 @@ def get_content(self, content_guid):
         return results
 
     def wait_for_task(
-        self, task_id, log_callback, abort_func=lambda: False, timeout=None, poll_wait=0.5, raise_on_error=True
+        self, task_id, log_callback, abort_func=lambda: False, timeout=get_timeout(), poll_wait=0.5, raise_on_error=True
     ):
 
         last_status = None
@@ -387,7 +387,7 @@ def __init__(
             token=token,
             secret=secret,
         )
-        self.setup_client(cookies, timeout)
+        self.setup_client(cookies)
 
     @classmethod
     def fromConnectServer(cls, connect_server, **kwargs):
@@ -485,11 +485,11 @@ def setup_remote_server(
         else:
             raise RSConnectException("Unable to infer Connect server type and setup server.")
 
-    def setup_client(self, cookies=None, timeout=30, **kwargs):
+    def setup_client(self, cookies=None, **kwargs):
         if isinstance(self.remote_server, RSConnectServer):
-            self.client = RSConnectClient(self.remote_server, cookies, timeout)
+            self.client = RSConnectClient(self.remote_server, cookies)
         elif isinstance(self.remote_server, PositServer):
-            self.client = PositClient(self.remote_server, timeout)
+            self.client = PositClient(self.remote_server)
         else:
             raise RSConnectException("Unable to infer Connect client.")
 
@@ -678,7 +678,7 @@ def check_server_capabilities(self, capability_functions):
     def upload_rstudio_bundle(self, prepare_deploy_result, bundle_size: int, contents):
         upload_url = prepare_deploy_result.presigned_url
         parsed_upload_url = urlparse(upload_url)
-        with S3Client("{}://{}".format(parsed_upload_url.scheme, parsed_upload_url.netloc), timeout=120) as s3_client:
+        with S3Client("{}://{}".format(parsed_upload_url.scheme, parsed_upload_url.netloc)) as s3_client:
             upload_result = s3_client.upload(
                 "{}?{}".format(parsed_upload_url.path, parsed_upload_url.query),
                 prepare_deploy_result.presigned_checksum,
@@ -1028,14 +1028,14 @@ class PositClient(HTTPServer):
 
     _TERMINAL_STATUSES = {"success", "failed", "error"}
 
-    def __init__(self, rstudio_server: PositServer, timeout: int = 30):
+    def __init__(self, rstudio_server: PositServer):
         self._token = rstudio_server.token
         try:
             self._key = base64.b64decode(rstudio_server.secret)
         except binascii.Error as e:
             raise RSConnectException("Invalid secret.") from e
         self._server = rstudio_server
-        super().__init__(rstudio_server.url, timeout=timeout)
+        super().__init__(rstudio_server.url)
 
     def _get_canonical_request(self, method, path, timestamp, content_hash):
         return "\n".join([method, path, timestamp, content_hash])
@@ -1121,7 +1121,7 @@ def get_task(self, task_id):
     def get_current_user(self):
         return self.get("/v1/users/me")
 
-    def wait_until_task_is_successful(self, task_id, timeout=180):
+    def wait_until_task_is_successful(self, task_id, timeout=get_timeout()):
         print()
         print("Waiting for task: {}".format(task_id))
         start_time = time.time()
@@ -1372,7 +1372,7 @@ def emit_task_log(
     task_id,
     log_callback,
     abort_func=lambda: False,
-    timeout=None,
+    timeout=get_timeout(),
     poll_wait=0.5,
     raise_on_error=True,
 ):

rsconnect/http_support.py

@@ -13,11 +13,13 @@
 from six.moves.urllib_parse import urlparse, urlencode, urljoin
 import base64
 
+from .timeouts import get_timeout
+
 _user_agent = "rsconnect-python/%s" % VERSION
 
 
 # noinspection PyUnusedLocal,PyUnresolvedReferences
-def _create_plain_connection(host_name, port, disable_tls_check, ca_data, timeout):
+def _create_plain_connection(host_name, port, disable_tls_check, ca_data):
     """
     This function is used to create a plain HTTP connection.  Note that the 3rd and 4th
     parameters are ignored; they are present to make the signature match the companion
@@ -27,9 +29,10 @@ def _create_plain_connection(host_name, port, disable_tls_check, ca_data, timeou
     :param port:  the port to connect to.
     :param disable_tls_check: notes whether TLS verification should be disabled (ignored).
     :param ca_data: any certificate authority information to use (ignored).
-    :param timeout: the timeout value to use for socket operations.
     :return: a plain HTTP connection.
     """
+    timeout = get_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)
 
 
@@ -59,7 +62,7 @@ def _get_proxy_headers(*args, **kwargs):
 
 
 # noinspection PyUnresolvedReferences
-def _create_ssl_connection(host_name, port, disable_tls_check, ca_data, timeout):
+def _create_ssl_connection(host_name, port, disable_tls_check, ca_data):
     """
     This function is used to create a TLS encrypted HTTP connection (SSL).
 
@@ -74,6 +77,8 @@ def _create_ssl_connection(host_name, port, disable_tls_check, ca_data, timeout)
         raise ValueError("Cannot both disable TLS checking and provide a custom certificate")
     _, _, proxyHost, proxyPort = _get_proxy()
     headers = _get_proxy_headers()
+    timeout = get_timeout()
+    logger.debug(f"The HTTPSConnection timeout is set to '{timeout}' seconds")
     if ca_data is not None:
         return http.HTTPSConnection(
             host_name,
@@ -162,7 +167,7 @@ class HTTPServer(object):
     server.
     """
 
-    def __init__(self, url, disable_tls_check=False, ca_data=None, cookies=None, timeout=30):
+    def __init__(self, url, disable_tls_check=False, ca_data=None, cookies=None):
         """
         Constructs an HTTPServer object.
 
@@ -174,7 +179,6 @@ def __init__(self, url, disable_tls_check=False, ca_data=None, cookies=None, tim
         certificates.
         :param cookies: an optional cookie jar.  Must be of type `CookieJar` defined in this
         same file (i.e., not the one Python provides).
-        :param timeout: the timeout value to use for socket operations.
         """
         self._url = urlparse(url)
 
@@ -184,7 +188,6 @@ def __init__(self, url, disable_tls_check=False, ca_data=None, cookies=None, tim
         self._disable_tls_check = disable_tls_check
         self._ca_data = ca_data
         self._cookies = cookies if cookies is not None else CookieJar()
-        self._timeout = timeout
         self._headers = {"User-Agent": _user_agent}
         self._conn = None
         self._proxy_headers = _get_proxy_headers()
@@ -216,7 +219,6 @@ def __enter__(self):
             self._url.port,
             self._disable_tls_check,
             self._ca_data,
-            self._timeout,
         )
         return self
 

rsconnect/timeouts.py

@@ -0,0 +1,41 @@
+import os
+from typing import Union
+
+from rsconnect.exception import RSConnectException
+
+_CONNECT_REQUEST_TIMEOUT_KEY = "CONNECT_REQUEST_TIMEOUT"
+_CONNECT_REQUEST_TIMEOUT_DEFAULT_VALUE = "300"
+
+
+def get_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.
+
+    If CONNECT_REQUEST_TIMEOUT is unset, a default value of 300 is used.
+
+    If CONNECT_REQUEST_TIMEOUT is set to a value less than 0, an `RSConnectException` is raised.
+
+    A CONNECT_REQUEST_TIMEOUT set to 0 is logically equivalent to no timeout.
+
+    The primary intent for this method is for usage with the `http` module. Specifically, for setting the timeout
+    parameter with an `http.client.HTTPConnection` or `http.client.HTTPSConnection`.
+
+    :raises: `RSConnectException` if CONNECT_REQUEST_TIMEOUT is not a natural number.
+    :return: the timeout value
+    """
+    timeout: Union[int, str] = os.environ.get(_CONNECT_REQUEST_TIMEOUT_KEY, _CONNECT_REQUEST_TIMEOUT_DEFAULT_VALUE)
+
+    try:
+        timeout = int(timeout)
+    except ValueError:
+        raise RSConnectException(
+            f"'CONNECT_REQUEST_TIMEOUT' is set to '{timeout}'. The value must be a natural number."
+        )
+
+    if timeout < 0:
+        raise RSConnectException(
+            f"'CONNECT_REQUEST_TIMEOUT' is set to '{timeout}'. The value must be a natural number."
+        )
+
+    return timeout

tests/test_http_support.py

@@ -19,7 +19,7 @@ def test_connection_factory_map(self):
 
     def test_create_ssl_checks(self):
         with self.assertRaises(ValueError):
-            _create_ssl_connection(None, None, True, "fake", 10)
+            _create_ssl_connection(None, None, True, "fake")
 
     def test_append_to_path(self):
         self.assertEqual(append_to_path("path/", "/sub"), "path/sub")

tests/test_timeouts.py

@@ -0,0 +1,33 @@
+import os
+
+from unittest import TestCase
+from unittest.mock import patch
+
+from rsconnect.exception import RSConnectException
+from rsconnect.timeouts import get_timeout
+
+
+class GetTimeoutTestCase(TestCase):
+    def test_get_default_timeout(self):
+        timeout = get_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()
+            self.assertEqual(24, timeout)
+
+    def test_get_zero_timeout_from_environment(self):
+        with patch.dict(os.environ, {"CONNECT_REQUEST_TIMEOUT": "0"}):
+            timeout = get_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()
+
+    def test_get_negative_timeout_from_environment(self):
+        with patch.dict(os.environ, {"CONNECT_REQUEST_TIMEOUT": "-24"}):
+            with self.assertRaises(RSConnectException):
+                get_timeout()