Adds a request timeout to the sync client, plus two small request fixes

README.md

@@ -38,6 +38,8 @@ print(dp_client.users.get(user_id='1234567'))
 - `sandbox (optional)` If the `sandbox` argument is set to `True`, then API calls will be
   routed to `https://sandbox.dialpad.com`.
 - `base_url (optional)` Routes requests to a specific url.
+- `timeout (optional)` The number of seconds to wait for a response before giving up on a request.
+  Defaults to `600`. Pass `None` to wait indefinitely.
 
 
 ### API Resources

src/dialpad/async_client.py

@@ -6,6 +6,9 @@
 
 hosts = dict(live='https://dialpad.com', sandbox='https://sandbox.dialpad.com')
 
+# How long to wait on the API before giving up on a request, in seconds.
+DEFAULT_TIMEOUT = 600.0
+
 
 class AsyncDialpadClient(AsyncDialpadResourcesMixin):
   def __init__(
@@ -14,9 +17,10 @@ def __init__(
     sandbox: bool = False,
     base_url: Optional[str] = None,
     company_id: Optional[str] = None,
+    timeout: Optional[float] = DEFAULT_TIMEOUT,
   ):
     self._token = token
-    self._session = httpx.AsyncClient(timeout=600.0)
+    self._session = httpx.AsyncClient(timeout=timeout)
     self._base_url = base_url or hosts.get('sandbox' if sandbox else 'live')
     self._company_id = company_id
 
@@ -30,7 +34,7 @@ def company_id(self, value):
 
   @company_id.deleter
   def company_id(self):
-    del self._company_id
+    self._company_id = None
 
   def _url(self, path: str) -> str:
     return f'{self._base_url}/{path.lstrip("/")}'
@@ -44,7 +48,8 @@ async def _raw_request(
     headers: Optional[dict] = None,
   ) -> httpx.Response:
     url = self._url(sub_path)
-    headers = headers or dict()
+    # Work on a copy, so that the caller's dict is not left holding the API token.
+    headers = dict(headers or {})
     if self.company_id:
       headers.update({'DP-Company-ID': str(self.company_id)})
 

src/dialpad/client.py

@@ -6,6 +6,9 @@
 
 hosts = dict(live='https://dialpad.com', sandbox='https://sandbox.dialpad.com')
 
+# How long to wait on the API before giving up on a request, in seconds.
+DEFAULT_TIMEOUT = 600.0
+
 
 class DialpadClient(DialpadResourcesMixin):
   def __init__(
@@ -14,9 +17,11 @@ def __init__(
     sandbox: bool = False,
     base_url: Optional[str] = None,
     company_id: Optional[str] = None,
+    timeout: Optional[float] = DEFAULT_TIMEOUT,
   ):
     self._token = token
     self._session = requests.Session()
+    self._timeout = timeout
     self._base_url = base_url or hosts.get('sandbox' if sandbox else 'live')
     self._company_id = company_id
 
@@ -30,7 +35,7 @@ def company_id(self, value):
 
   @company_id.deleter
   def company_id(self):
-    del self._company_id
+    self._company_id = None
 
   def _url(self, path: str) -> str:
     return f'{self._base_url}/{path.lstrip("/")}'
@@ -44,7 +49,8 @@ def _raw_request(
     headers: Optional[dict] = None,
   ) -> requests.Response:
     url = self._url(sub_path)
-    headers = headers or dict()
+    # Work on a copy, so that the caller's dict is not left holding the API token.
+    headers = dict(headers or {})
     if self.company_id:
       headers.update({'DP-Company-ID': str(self.company_id)})
 
@@ -55,6 +61,7 @@ def _raw_request(
         headers=headers,
         params=params,
         json=body,
+        timeout=self._timeout,
       )
     raise ValueError('Unsupported method "%s"' % method)
 

test/test_client_requests.py

@@ -0,0 +1,121 @@
+#!/usr/bin/env python
+
+"""Tests for the request plumbing that every resource method goes through.
+
+In particular these tests ensure:
+
+- Requests are sent with a timeout, so that an unanswered request cannot block the caller forever
+- A headers dict supplied by the caller is not modified (and so never ends up holding the API token)
+- The company_id can be deleted without breaking subsequent requests
+"""
+
+import httpx
+import pytest
+
+from dialpad import async_client, client
+from dialpad.async_client import AsyncDialpadClient
+from dialpad.client import DialpadClient
+
+USER_PATH = '/api/v2/users/1'
+USER_URL = 'https://dialpad.com' + USER_PATH
+
+
+class TestClientRequests:
+  """Verifies the request handling of the DialpadClient."""
+
+  def test_default_timeout(self, requests_mock):
+    """Verifies that requests are sent with a timeout by default."""
+    requests_mock.get(USER_URL, json={'id': 1})
+
+    DialpadClient('123').users.get('1')
+
+    assert requests_mock.last_request.timeout == client.DEFAULT_TIMEOUT
+
+  def test_custom_timeout(self, requests_mock):
+    """Verifies that the timeout argument is respected, including None to disable it."""
+    requests_mock.get(USER_URL, json={'id': 1})
+
+    DialpadClient('123', timeout=5).users.get('1')
+    assert requests_mock.last_request.timeout == 5
+
+    DialpadClient('123', timeout=None).users.get('1')
+    assert requests_mock.last_request.timeout is None
+
+  def test_caller_headers_are_not_modified(self, requests_mock):
+    """Verifies that the auth headers are not written into the caller's headers dict."""
+    requests_mock.get(USER_URL, json={'id': 1})
+    headers = {'X-Trace': 'abc'}
+
+    dp = DialpadClient('123', company_id='42')
+    dp.request(method='GET', sub_path=USER_PATH, headers=headers)
+
+    assert headers == {'X-Trace': 'abc'}
+
+    sent_headers = requests_mock.last_request.headers
+    assert sent_headers['X-Trace'] == 'abc'
+    assert sent_headers['Authorization'] == 'Bearer 123'
+    assert sent_headers['DP-Company-ID'] == '42'
+
+  def test_company_id_can_be_deleted(self, requests_mock):
+    """Verifies that requests still work after the company_id has been deleted."""
+    requests_mock.get(USER_URL, json={'id': 1})
+
+    dp = DialpadClient('123', company_id='42')
+    del dp.company_id
+
+    assert dp.company_id is None
+    dp.users.get('1')
+    assert 'DP-Company-ID' not in requests_mock.last_request.headers
+
+
+class TestAsyncClientRequests:
+  """Verifies the request handling of the AsyncDialpadClient."""
+
+  @pytest.mark.asyncio
+  async def test_default_timeout(self, httpx_mock):
+    """Verifies that requests are sent with a timeout by default."""
+    httpx_mock.add_response(url=USER_URL, json={'id': 1})
+
+    await AsyncDialpadClient('123').users.get('1')
+
+    expected = httpx.Timeout(async_client.DEFAULT_TIMEOUT).as_dict()
+    assert httpx_mock.get_request().extensions['timeout'] == expected
+
+  @pytest.mark.asyncio
+  async def test_custom_timeout(self, httpx_mock):
+    """Verifies that the timeout argument is respected, including None to disable it."""
+    httpx_mock.add_response(url=USER_URL, json={'id': 1}, is_reusable=True)
+
+    await AsyncDialpadClient('123', timeout=5).users.get('1')
+    assert httpx_mock.get_requests()[-1].extensions['timeout'] == httpx.Timeout(5).as_dict()
+
+    await AsyncDialpadClient('123', timeout=None).users.get('1')
+    assert httpx_mock.get_requests()[-1].extensions['timeout'] == httpx.Timeout(None).as_dict()
+
+  @pytest.mark.asyncio
+  async def test_caller_headers_are_not_modified(self, httpx_mock):
+    """Verifies that the auth headers are not written into the caller's headers dict."""
+    httpx_mock.add_response(url=USER_URL, json={'id': 1})
+    headers = {'X-Trace': 'abc'}
+
+    dp = AsyncDialpadClient('123', company_id='42')
+    await dp.request(method='GET', sub_path=USER_PATH, headers=headers)
+
+    assert headers == {'X-Trace': 'abc'}
+
+    sent_headers = httpx_mock.get_request().headers
+    assert sent_headers['X-Trace'] == 'abc'
+    assert sent_headers['Authorization'] == 'Bearer 123'
+    assert sent_headers['DP-Company-ID'] == '42'
+
+  @pytest.mark.asyncio
+  async def test_company_id_can_be_deleted(self, httpx_mock):
+    """Verifies that requests still work after the company_id has been deleted."""
+    httpx_mock.add_response(url=USER_URL, json={'id': 1})
+
+    dp = AsyncDialpadClient('123', company_id='42')
+    del dp.company_id
+
+    assert dp.company_id is None
+    await dp.users.get('1')
+    assert 'DP-Company-ID' not in httpx_mock.get_request().headers