Merge branch 'master' of github.com:biothings/biothings_client.py

Author: jschaff

biothings_client/__init__.py

@@ -6,13 +6,15 @@
 from biothings_client.client.base import BiothingClient, get_client
 from biothings_client.__version__ import __version__
 from biothings_client._dependencies import _CACHING, _PANDAS
+from biothings_client.utils._external import alwayslist
 
 __all__ = [
     "AsyncBiothingClient",
     "BiothingClient",
     "_CACHING",
     "_PANDAS",
     "__version__",
+    "alwayslist",
     "get_async_client",
     "get_client",
 ]

biothings_client/client/asynchronous.py

@@ -48,6 +48,9 @@
 logger.setLevel(logging.INFO)
 
 
+PROXY_MOUNT = Dict[str, Union[httpx.BaseTransport, None]]
+
+
 # Future work:
 # Consider use "verbose" settings to control default logging output level
 # by doing this instead of using branching throughout the application,
@@ -100,6 +103,12 @@ async def _build_http_client(self, cache_db: Union[str, Path] = None) -> None:
         This modifies the state of the BiothingsClient instance
         to set the values for the http_client property
 
+        For the moment, we have disabled timeouts. This matches our prior
+        behavior we had with the requests library, which by default did not
+        specify a timeout when making a request. In the future this should
+        be modified to prevent indefinite hanging with potentially bad network
+        connections
+
         Inputs:
         :param cache_db: pathlike object to the local sqlite3 cache database file
 
@@ -108,6 +117,7 @@ async def _build_http_client(self, cache_db: Union[str, Path] = None) -> None:
         """
         if not self.http_client_setup:
             self.http_client = httpx.AsyncClient()
+            self.http_client = httpx.AsyncClient(timeout=None)
             self.http_client_setup = True
             self.http_cache_client_setup = False
 
@@ -121,6 +131,12 @@ async def _build_cache_http_client(self, cache_db: Union[str, Path] = None) -> N
         This modifies the state of the BiothingsClient instance
         to set the values for the http_client property and the cache_storage property
 
+        For the moment, we have disabled timeouts. This matches our prior
+        behavior we had with the requests library, which by default did not
+        specify a timeout when making a request. In the future this should
+        be modified to prevent indefinite hanging with potentially bad network
+        connections
+
         Inputs:
         :param cache_db: pathlike object to the local sqlite3 cache database file
 
@@ -134,15 +150,47 @@ async def _build_cache_http_client(self, cache_db: Union[str, Path] = None) -> N
 
             self.cache_storage = AsyncBiothingsClientSqlite3Cache()
             await self.cache_storage.setup_database_connection(cache_db)
-            cache_transport = hishel.AsyncCacheTransport(
-                transport=httpx.AsyncHTTPTransport(), storage=self.cache_storage
-            )
+
+            async_http_transport = httpx.AsyncHTTPTransport()
+            cache_transport = hishel.AsyncCacheTransport(transport=async_http_transport, storage=self.cache_storage)
             cache_controller = hishel.Controller(cacheable_methods=["GET", "POST"])
+
+            # Have to manually build the proxy mounts as httpx will not auto-discover
+            # proxies if we provide our own HTTPTransport to the Client constructor
+            proxy_mounts = self._build_caching_proxy_mounts()
             self.http_client = hishel.AsyncCacheClient(
-                controller=cache_controller, transport=cache_transport, storage=self.cache_storage
+                controller=cache_controller,
+                transport=cache_transport,
+                storage=self.cache_storage,
+                mounts=proxy_mounts,
+                timeout=None,
             )
             self.http_client_setup = True
 
+    def _build_caching_proxy_mounts(self) -> PROXY_MOUNT:
+        """
+        Builds the proxy mounts for case where we have a CacheTransport.
+        Autodiscovery of proxies only works when don't provide a transport
+        to the client so this method acts as a replacement for that
+        """
+        proxy_map = httpx._utils.get_environment_proxies()
+        proxy_mounts: PROXY_MOUNT = {}
+        for key, proxy in proxy_map.items():
+            proxy_transport = None
+            if proxy is not None:
+                proxy_transport = httpx.AsyncHTTPTransport(
+                    verify=True,
+                    cert=None,
+                    trust_env=True,
+                    http1=True,
+                    http2=False,
+                    limits=httpx._config.DEFAULT_LIMITS,
+                    proxy=proxy,
+                )
+            proxy_mounts[key] = proxy_transport
+        proxy_mounts = dict(sorted(proxy_mounts.items()))
+        return proxy_mounts
+
     async def __del__(self):
         """
         Destructor for the client to ensure that we close any potential

biothings_client/client/base.py

@@ -49,6 +49,9 @@
 logger.setLevel(logging.INFO)
 
 
+PROXY_MOUNT = Dict[str, Union[httpx.BaseTransport, None]]
+
+
 # Future work:
 # Consider use "verbose" settings to control default logging output level
 # by doing this instead of using branching throughout the application,
@@ -103,9 +106,15 @@ def _build_http_client(self, cache_db: Union[str, Path] = None) -> None:
 
         This modifies the state of the BiothingsClient instance
         to set the values for the http_client property
+
+        For the moment, we have disabled timeouts. This matches our prior
+        behavior we had with the requests library, which by default did not
+        specify a timeout when making a request. In the future this should
+        be modified to prevent indefinite hanging with potentially bad network
+        connections
         """
         if not self.http_client_setup:
-            self.http_client = httpx.Client()
+            self.http_client = httpx.Client(timeout=None)
             self.http_client_setup = True
             self.http_cache_client_setup = False
 
@@ -118,6 +127,12 @@ def _build_cache_http_client(self, cache_db: Union[str, Path] = None) -> None:
 
         This modifies the state of the BiothingsClient instance
         to set the values for the http_client property and the cache_storage property
+
+        For the moment, we have disabled timeouts. This matches our prior
+        behavior we had with the requests library, which by default did not
+        specify a timeout when making a request. In the future this should
+        be modified to prevent indefinite hanging with potentially bad network
+        connections
         """
         if not self.http_client_setup:
             if cache_db is None:
@@ -126,13 +141,47 @@ def _build_cache_http_client(self, cache_db: Union[str, Path] = None) -> None:
 
             self.cache_storage = BiothingsClientSqlite3Cache()
             self.cache_storage.setup_database_connection(cache_db)
-            cache_transport = hishel.CacheTransport(transport=httpx.HTTPTransport(), storage=self.cache_storage)
+
+            http_transport = httpx.HTTPTransport()
+            cache_transport = hishel.CacheTransport(transport=http_transport, storage=self.cache_storage)
             cache_controller = hishel.Controller(cacheable_methods=["GET", "POST"])
+
+            # Have to manually build the proxy mounts as httpx will not auto-discover
+            # proxies if we provide our own HTTPTransport to the Client constructor
+            proxy_mounts = self._build_caching_proxy_mounts()
             self.http_client = hishel.CacheClient(
-                controller=cache_controller, transport=cache_transport, storage=self.cache_storage
+                controller=cache_controller,
+                transport=cache_transport,
+                storage=self.cache_storage,
+                mounts=proxy_mounts,
+                timeout=None,
             )
             self.http_client_setup = True
 
+    def _build_caching_proxy_mounts(self) -> PROXY_MOUNT:
+        """
+        Builds the proxy mounts for case where we have a CacheTransport.
+        Autodiscovery of proxies only works when don't provide a transport
+        to the client so this method acts as a replacement for that
+        """
+        proxy_map = httpx._utils.get_environment_proxies()
+        proxy_mounts: PROXY_MOUNT = {}
+        for key, proxy in proxy_map.items():
+            proxy_transport = None
+            if proxy is not None:
+                proxy_transport = httpx.HTTPTransport(
+                    verify=True,
+                    cert=None,
+                    trust_env=True,
+                    http1=True,
+                    http2=False,
+                    limits=httpx._config.DEFAULT_LIMITS,
+                    proxy=proxy,
+                )
+            proxy_mounts[key] = proxy_transport
+        proxy_mounts = dict(sorted(proxy_mounts.items()))
+        return proxy_mounts
+
     def __del__(self):
         """
         Destructor for the client to ensure that we close any potential

biothings_client/utils/_external.py

@@ -0,0 +1,36 @@
+"""
+Any functions we provide as auxillary or helper
+functions for the client are stored here and exposed in the
+__init__ for usage by the users
+
+We likely don't use these throughout the biothings.api core
+code, but the users might so we want to maintain that here while
+also indicating the purposes of the collection of functions in the
+module
+"""
+
+from typing import Any, Union
+
+
+def alwayslist(value: Any) -> Union[list, tuple]:
+    """
+    Simple transformation function that ensures the output is an iterable.
+
+    Control Flow:
+    If the input value is a {list, tuple}, we no-opt and return the value unchanged
+    Otherwise, we wrap the value in a list and then return that list
+
+    Example:
+
+    >>> x = 'abc'
+    >>> for xx in alwayslist(x):
+    ...     print xx
+    >>> x = ['abc', 'def']
+    >>> for xx in alwayslist(x):
+    ...     print xx
+
+    """
+    if isinstance(value, (list, tuple)):
+        return value
+    else:
+        return [value]

tests/test_async.py

@@ -115,3 +115,41 @@ async def test_async_client_proxy_discovery(mock_client_proxy_configuration):
             assert proxy_url.host == b"fakehttpproxyhost"
             assert proxy_url.port == 6374
             assert proxy_url.target == b"/"
+
+
+@pytest.mark.asyncio
+@pytest.mark.skipif(not biothings_client._CACHING, reason="caching libraries not installed")
+async def test_async_cache_client_proxy_discovery(mock_client_proxy_configuration):
+    """
+    Tests for verifying that we properly auto-discover the
+    proxy configuration from the environment using the built-in
+    methods provided by HTTPX
+
+    Brought to light by user issues on VPN
+    https://github.com/biothings/mygene.py/issues/26#issuecomment-2588065562
+    """
+    client_name = "gene"
+    gene_client = biothings_client.get_async_client(client_name)
+    await gene_client._build_cache_http_client()
+
+    http_mounts = gene_client.http_client._mounts
+    assert isinstance(http_mounts, dict)
+    assert len(http_mounts) == 2
+
+    for url_pattern, http_transport in gene_client.http_client._mounts.items():
+        assert isinstance(url_pattern, httpx._utils.URLPattern)
+        assert isinstance(http_transport, httpx.AsyncHTTPTransport)
+
+        if url_pattern.pattern == "https://":
+            proxy_url = http_transport._pool._proxy_url
+            assert proxy_url.scheme == b"http"
+            assert proxy_url.host == b"fakehttpsproxyhost"
+            assert proxy_url.port == 6375
+            assert proxy_url.target == b"/"
+
+        elif url_pattern.pattern == "http://":
+            proxy_url = http_transport._pool._proxy_url
+            assert proxy_url.scheme == b"http"
+            assert proxy_url.host == b"fakehttpproxyhost"
+            assert proxy_url.port == 6374
+            assert proxy_url.target == b"/"

tests/test_sync.py

@@ -94,6 +94,47 @@ def test_client_proxy_discovery(mock_client_proxy_configuration):
     client_name = "gene"
     gene_client = biothings_client.get_client(client_name)
     gene_client._build_http_client()
+    http_mounts = gene_client.http_client._mounts
+    assert isinstance(http_mounts, dict)
+    assert len(http_mounts) == 2
+
+    for url_pattern, http_transport in http_mounts.items():
+        assert isinstance(url_pattern, httpx._utils.URLPattern)
+        assert isinstance(http_transport, httpx.HTTPTransport)
+
+        if url_pattern.pattern == "https://":
+            proxy_url = http_transport._pool._proxy_url
+            assert proxy_url.scheme == b"http"
+            assert proxy_url.host == b"fakehttpsproxyhost"
+            assert proxy_url.port == 6375
+            assert proxy_url.target == b"/"
+
+        elif url_pattern.pattern == "http://":
+            proxy_url = http_transport._pool._proxy_url
+            assert proxy_url.scheme == b"http"
+            assert proxy_url.host == b"fakehttpproxyhost"
+            assert proxy_url.port == 6374
+            assert proxy_url.target == b"/"
+
+
+@pytest.mark.skipif(not biothings_client._CACHING, reason="caching libraries not installed")
+def test_cache_client_proxy_discovery(mock_client_proxy_configuration):
+    """
+    Tests for verifying that we properly auto-discover the
+    proxy configuration from the environment using the built-in
+    methods provided by HTTPX
+
+    Brought to light by user issues on VPN
+    https://github.com/biothings/mygene.py/issues/26#issuecomment-2588065562
+    """
+    client_name = "gene"
+    gene_client = biothings_client.get_client(client_name)
+    gene_client._build_cache_http_client()
+
+    http_mounts = gene_client.http_client._mounts
+    assert isinstance(http_mounts, dict)
+    assert len(http_mounts) == 2
+
     for url_pattern, http_transport in gene_client.http_client._mounts.items():
         assert isinstance(url_pattern, httpx._utils.URLPattern)
         assert isinstance(http_transport, httpx.HTTPTransport)