Merge pull request #102 from 1Password/fix/timeout

Fix/timeout

src/onepasswordconnectsdk/async_client.py

@@ -24,7 +24,6 @@ def __init__(self, url: str, token: str) -> None:
         self.serializer = Serializer()
 
     def create_session(self, url: str, token: str) -> httpx.AsyncClient:
-        # import here to avoid circular import
         return httpx.AsyncClient(base_url=url, headers=self.build_headers(token), timeout=get_timeout())
 
     def build_headers(self, token: str) -> Dict[str, str]:

src/onepasswordconnectsdk/utils.py

@@ -1,8 +1,6 @@
 import os
-from typing import Union
 
-from httpx import USE_CLIENT_DEFAULT
-from httpx._client import UseClientDefault
+from httpx._client import DEFAULT_TIMEOUT_CONFIG, Timeout
 
 UUIDLength = 26
 ENV_CLIENT_REQUEST_TIMEOUT = "OP_CONNECT_CLIENT_REQ_TIMEOUT"
@@ -68,7 +66,13 @@ def _append_path(self, path_chunk: str = None, query: str = None) -> 'PathBuilde
             self.path += f"?{query}"
 
 
-def get_timeout() -> Union[int, UseClientDefault]:
+def get_timeout() -> Timeout:
     """Get the timeout to be used in the HTTP Client"""
-    timeout = int(os.getenv(ENV_CLIENT_REQUEST_TIMEOUT, 0))
-    return timeout if timeout else USE_CLIENT_DEFAULT
+    raw_timeout = os.getenv(ENV_CLIENT_REQUEST_TIMEOUT, '0.0')
+    if raw_timeout == 'None':
+        return Timeout(None)  # disable all timeouts
+    elif raw_timeout.isnumeric():
+        timeout = float(raw_timeout)
+        return timeout if timeout else DEFAULT_TIMEOUT_CONFIG
+    else:
+        return DEFAULT_TIMEOUT_CONFIG

src/tests/test_client_items.py

@@ -3,6 +3,7 @@
 from unittest import mock
 
 from httpx import Response
+from httpx._client import DEFAULT_TIMEOUT_CONFIG
 from onepasswordconnectsdk import client, models
 from onepasswordconnectsdk.utils import ENV_CLIENT_REQUEST_TIMEOUT
 
@@ -446,6 +447,11 @@ def generate_full_item():
     return item
 
 
+def test_default_timeout():
+    client_instance = client.new_client(HOST, TOKEN)
+    assert client_instance.session.timeout.read == DEFAULT_TIMEOUT_CONFIG.read
+
+
 def test_set_timeout_using_env_variable():
     with mock.patch.dict(os.environ, {ENV_CLIENT_REQUEST_TIMEOUT: '120'}):
         client_instance = client.new_client(HOST, TOKEN)
@@ -456,4 +462,40 @@ def test_set_timeout_using_env_variable():
 def test_set_timeout_using_env_variable_async():
     with mock.patch.dict(os.environ, {ENV_CLIENT_REQUEST_TIMEOUT: '120'}):
         client_instance = client.new_client(HOST, TOKEN, is_async=True)
-        assert client_instance.session.timeout.read == 120
+        assert client_instance.session.timeout.read == 120
+
+
+def test_disable_all_timeouts():
+    with mock.patch.dict(os.environ, {ENV_CLIENT_REQUEST_TIMEOUT: 'None'}):
+        client_instance = client.new_client(HOST, TOKEN)
+        assert client_instance.session.timeout.read is None
+
+
+def test_env_client_request_timeout_env_var_is_empty_string():
+    with mock.patch.dict(os.environ, {ENV_CLIENT_REQUEST_TIMEOUT: ''}):
+        client_instance = client.new_client(HOST, TOKEN)
+        assert client_instance.session.timeout.read == DEFAULT_TIMEOUT_CONFIG.read
+
+
+def test_env_client_request_timeout_env_var_is_single_space_string():
+    with mock.patch.dict(os.environ, {ENV_CLIENT_REQUEST_TIMEOUT: ' '}):
+        client_instance = client.new_client(HOST, TOKEN)
+        assert client_instance.session.timeout.read == DEFAULT_TIMEOUT_CONFIG.read
+
+
+def test_env_client_request_timeout_env_var_is_not_numeric_string():
+    with mock.patch.dict(os.environ, {ENV_CLIENT_REQUEST_TIMEOUT: 'abc'}):
+        client_instance = client.new_client(HOST, TOKEN)
+        assert client_instance.session.timeout.read == DEFAULT_TIMEOUT_CONFIG.read
+
+
+def test_env_client_request_timeout_env_var_is_zero():
+    with mock.patch.dict(os.environ, {ENV_CLIENT_REQUEST_TIMEOUT: '0'}):
+        client_instance = client.new_client(HOST, TOKEN)
+        assert client_instance.session.timeout.read == DEFAULT_TIMEOUT_CONFIG.read
+
+
+def test_env_client_request_timeout_env_var_is_negative_number():
+    with mock.patch.dict(os.environ, {ENV_CLIENT_REQUEST_TIMEOUT: '-10'}):
+        client_instance = client.new_client(HOST, TOKEN)
+        assert client_instance.session.timeout.read == DEFAULT_TIMEOUT_CONFIG.read