Repository: encode/httpx
Pull request #1065 (2020-07-17 09:54:44)

More public API tightening
Move the following into private API space...

* `.get_proxy_map`
* `.init_transport`
* `.init_proxy_transport`
* `.proxies`
* `.transport`

I believe I omitted these in #997 because some of the proxy test cases call into them, which meant these were less clear than the cases in that PR, but I figure we really do want them private in any case, and if we've got some test cases that track a bit of internal state, then well okay, let's live with that for now.

There's also a few attributes that are set in `BaseClient.__init__` that look like contenders here, but I think we should treat those as a separate PR, so...

* `self.base_url` - Potentially modify this to a property setter / getter style, but not 100% obvious?
* `self.auth` - Potentially modify this to a property setter / getter style, but not 100% obvious?
* `self.timeout` - Looks like it should *definitely* move to a property getter / setter style, as per params/headers/cookies.
* `self.max_redirects` - I think this one is okay as a public attribute, since it's just a plain int.
* `self.trust_env` - Setting this after `__init__` isn't really valid (eg. the transport is already initialized) so we could move it to a read-only property?
* `self.netrc` - Ought to be private.


Patch:
diff --git a/httpx/_client.py b/httpx/_client.py
index bea0f73..dae1503 100644
--- a/httpx/_client.py
+++ b/httpx/_client.py
@@ -82,7 +82,7 @@ class BaseClient:
         self.trust_env = trust_env
         self.netrc = NetRCInfo()
 
-    def get_proxy_map(
+    def _get_proxy_map(
         self, proxies: typing.Optional[ProxiesTypes], trust_env: bool,
     ) -> typing.Dict[str, Proxy]:
         if proxies is None:
@@ -454,9 +454,9 @@ class Client(BaseClient):
             trust_env=trust_env,
         )
 
-        proxy_map = self.get_proxy_map(proxies, trust_env)
+        proxy_map = self._get_proxy_map(proxies, trust_env)
 
-        self.transport = self.init_transport(
+        self._transport = self._init_transport(
             verify=verify,
             cert=cert,
             http2=http2,
@@ -465,8 +465,8 @@ class Client(BaseClient):
             app=app,
             trust_env=trust_env,
         )
-        self.proxies: typing.Dict[str, httpcore.SyncHTTPTransport] = {
-            key: self.init_proxy_transport(
+        self._proxies: typing.Dict[str, httpcore.SyncHTTPTransport] = {
+            key: self._init_proxy_transport(
                 proxy,
                 verify=verify,
                 cert=cert,
@@ -477,7 +477,7 @@ class Client(BaseClient):
             for key, proxy in proxy_map.items()
         }
 
-    def init_transport(
+    def _init_transport(
         self,
         verify: VerifyTypes = True,
         cert: CertTypes = None,
@@ -505,7 +505,7 @@ class Client(BaseClient):
             http2=http2,
         )
 
-    def init_proxy_transport(
+    def _init_proxy_transport(
         self,
         proxy: Proxy,
         verify: VerifyTypes = True,
@@ -529,12 +529,12 @@ class Client(BaseClient):
             http2=http2,
         )
 
-    def transport_for_url(self, url: URL) -> httpcore.SyncHTTPTransport:
+    def _transport_for_url(self, url: URL) -> httpcore.SyncHTTPTransport:
         """
         Returns the transport instance that should be used for a given URL.
         This will either be the standard connection pool, or a proxy.
         """
-        if self.proxies and not should_not_be_proxied(url):
+        if self._proxies and not should_not_be_proxied(url):
             is_default_port = (url.scheme == "http" and url.port == 80) or (
                 url.scheme == "https" and url.port == 443
             )
@@ -548,11 +548,11 @@ class Client(BaseClient):
                 "all",
             )
             for proxy_key in proxy_keys:
-                if proxy_key and proxy_key in self.proxies:
-                    transport = self.proxies[proxy_key]
+                if proxy_key and proxy_key in self._proxies:
+                    transport = self._proxies[proxy_key]
                     return transport
 
-        return self.transport
+        return self._transport
 
     def request(
         self,
@@ -684,7 +684,7 @@ class Client(BaseClient):
         Sends a single request, without handling any redirections.
         """
 
-        transport = self.transport_for_url(request.url)
+        transport = self._transport_for_url(request.url)
 
         try:
             with map_exceptions(HTTPCORE_EXC_MAP):
@@ -897,8 +897,8 @@ class Client(BaseClient):
         )
 
     def close(self) -> None:
-        self.transport.close()
-        for proxy in self.proxies.values():
+        self._transport.close()
+        for proxy in self._proxies.values():
             proxy.close()
 
     def __enter__(self) -> "Client":
@@ -992,9 +992,9 @@ class AsyncClient(BaseClient):
             trust_env=trust_env,
         )
 
-        proxy_map = self.get_proxy_map(proxies, trust_env)
+        proxy_map = self._get_proxy_map(proxies, trust_env)
 
-        self.transport = self.init_transport(
+        self._transport = self._init_transport(
             verify=verify,
             cert=cert,
             http2=http2,
@@ -1003,8 +1003,8 @@ class AsyncClient(BaseClient):
             app=app,
             trust_env=trust_env,
         )
-        self.proxies: typing.Dict[str, httpcore.AsyncHTTPTransport] = {
-            key: self.init_proxy_transport(
+        self._proxies: typing.Dict[str, httpcore.AsyncHTTPTransport] = {
+            key: self._init_proxy_transport(
                 proxy,
                 verify=verify,
                 cert=cert,
@@ -1015,7 +1015,7 @@ class AsyncClient(BaseClient):
             for key, proxy in proxy_map.items()
         }
 
-    def init_transport(
+    def _init_transport(
         self,
         verify: VerifyTypes = True,
         cert: CertTypes = None,
@@ -1043,7 +1043,7 @@ class AsyncClient(BaseClient):
             http2=http2,
         )
 
-    def init_proxy_transport(
+    def _init_proxy_transport(
         self,
         proxy: Proxy,
         verify: VerifyTypes = True,
@@ -1067,12 +1067,12 @@ class AsyncClient(BaseClient):
             http2=http2,
         )
 
-    def transport_for_url(self, url: URL) -> httpcore.AsyncHTTPTransport:
+    def _transport_for_url(self, url: URL) -> httpcore.AsyncHTTPTransport:
         """
         Returns the transport instance that should be used for a given URL.
         This will either be the standard connection pool, or a proxy.
         """
-        if self.proxies and not should_not_be_proxied(url):
+        if self._proxies and not should_not_be_proxied(url):
             is_default_port = (url.scheme == "http" and url.port == 80) or (
                 url.scheme == "https" and url.port == 443
             )
@@ -1086,11 +1086,11 @@ class AsyncClient(BaseClient):
                 "all",
             )
             for proxy_key in proxy_keys:
-                if proxy_key and proxy_key in self.proxies:
-                    transport = self.proxies[proxy_key]
+                if proxy_key and proxy_key in self._proxies:
+                    transport = self._proxies[proxy_key]
                     return transport
 
-        return self.transport
+        return self._transport
 
     async def request(
         self,
@@ -1225,7 +1225,7 @@ class AsyncClient(BaseClient):
         Sends a single request, without handling any redirections.
         """
 
-        transport = self.transport_for_url(request.url)
+        transport = self._transport_for_url(request.url)
 
         try:
             with map_exceptions(HTTPCORE_EXC_MAP):
@@ -1438,8 +1438,8 @@ class AsyncClient(BaseClient):
         )
 
     async def aclose(self) -> None:
-        await self.transport.aclose()
-        for proxy in self.proxies.values():
+        await self._transport.aclose()
+        for proxy in self._proxies.values():
             await proxy.aclose()
 
     async def __aenter__(self) -> "AsyncClient":


Test patch:
diff --git a/tests/client/test_proxies.py b/tests/client/test_proxies.py
index fb21760..f981948 100644
--- a/tests/client/test_proxies.py
+++ b/tests/client/test_proxies.py
@@ -24,12 +24,12 @@ def test_proxies_parameter(proxies, expected_proxies):
     client = httpx.AsyncClient(proxies=proxies)
 
     for proxy_key, url in expected_proxies:
-        assert proxy_key in client.proxies
-        proxy = client.proxies[proxy_key]
+        assert proxy_key in client._proxies
+        proxy = client._proxies[proxy_key]
         assert isinstance(proxy, httpcore.AsyncHTTPProxy)
         assert proxy.proxy_origin == httpx.URL(url).raw[:3]
 
-    assert len(expected_proxies) == len(client.proxies)
+    assert len(expected_proxies) == len(client._proxies)
 
 
 PROXY_URL = "http://[::1]"
@@ -79,10 +79,10 @@ PROXY_URL = "http://[::1]"
 )
 def test_transport_for_request(url, proxies, expected):
     client = httpx.AsyncClient(proxies=proxies)
-    transport = client.transport_for_url(httpx.URL(url))
+    transport = client._transport_for_url(httpx.URL(url))
 
     if expected is None:
-        assert transport is client.transport
+        assert transport is client._transport
     else:
         assert isinstance(transport, httpcore.AsyncHTTPProxy)
         assert transport.proxy_origin == httpx.URL(expected).raw[:3]
@@ -126,9 +126,9 @@ def test_proxies_environ(monkeypatch, client_class, url, env, expected):
         monkeypatch.setenv(name, value)
 
     client = client_class()
-    transport = client.transport_for_url(httpx.URL(url))
+    transport = client._transport_for_url(httpx.URL(url))
 
     if expected is None:
-        assert transport == client.transport
+        assert transport == client._transport
     else:
         assert transport.proxy_origin == httpx.URL(expected).raw[:3]