Repository: encode/httpx
Pull request: Drop urllib3 in favor of public gist
Fixes #1143 

This PR removes the `URLLib3Transport` from HTTPX, and moves it to a public gist that we're now pointing to in the "Custom transports" docs.

Also adds a small additional section to the Requests compatibility guide, so that people know this option exists if they're migrating from Requests to HTTPX.

Problem statement:
Move URLLib3Transport out of core and into a stand-alone package.
We should remove `URLLib3Transport` from `httpx`, and make it a stand-alone package, with a simple README documenting how to install and use it, with `transport=...`.

In fact, we *could* even just drop it from core, and *aim* at delivering a third party package but *not* treat having to do so as a 1.0 release blocker. (I mean, is anyone *actually* using it at the moment?)

Hints from issue discussion:
I think we could pretty much get away with just a GitHub gist implementation, that we can then point to in the docs as an example of a custom transport, using URLLib3.

Things to do here would be to make sure everything in the implementation is using public API, so...

* Drop `verify`, `cert`, `trust_env`, and instead use `ssl_context: SSLContext`, and direct users towards eg `ssl_context=httpx.create_ssl_context()` when instantiating the transport.
* Import NetworkError from the top level of `httpcore`... `httpcore.NetworkError`, *not* the `httpx` import we're currently using.
* (Re)Implement the `map_exceptions` helper locally. (From https://github.com/encode/httpx/blob/master/httpx/_exceptions.py#L336-L359 but we *won't* need the `**kwargs` cleverness in this case.) *OR* just drop `map_exceptions` and instead explicitly handle the catch and re-raise cases. (Probably simpler here, actually)
* Use `httpcore.PlainByteStream` and `httpcore.IteratorByteStream` rather than the content streams.

And also, a nice to have...

* Ensure retries is configured off, and drop the `MaxRetryError` case, since we won't need it, and anyways we'd rather eventually push towards client-configured retries, rather than transport-configured retries.
What do we think of this…? :-)

https://gist.github.com/florimondmanca/d56764d78d748eb9f73165da388e546e
Love it, yes! *Really* nice.

Couple of possible polishing touches...

* I don't think we need to import/catch MaxRetries, since we're using `retries=0`. In any case it's likely only ended up there as a remanent of having looked at how requests wraps up urllib3 exceptions.
* Perhaps returning a `SyncByteStream` subclass, `URLLib3ByteStream(conn=...)` would be more graceful than what we've been doing until now with the inner `response_bytes` function and the `close_func=conn.release_conn` callback?
* Maybe we should use `urllib3.exceptions.SSLError` rather than the extra import?

Anyways looks *ace* either with or without those. 👍
Very nice suggestions — amended! :tada:

Commit message: Merge branch 'master' into fm/urllib3-gist

Patch to review:
diff --git a/docs/advanced.md b/docs/advanced.md
index e43ecca6df..b2a07df371 100644
--- a/docs/advanced.md
+++ b/docs/advanced.md
@@ -809,6 +809,8 @@ HTTPX's `Client` also accepts a `transport` argument. This argument allows you
 to provide a custom Transport object that will be used to perform the actual
 sending of the requests.
 
+### Usage
+
 For some advanced configuration you might need to instantiate a transport
 class directly, and pass it to the client instance. The `httpcore` package
 provides a `local_address` configuration that is only available via this
@@ -850,18 +852,19 @@ do not include any default values for configuring aspects such as the
 connection pooling details, so you'll need to provide more explicit
 configuration when using this API.
 
-HTTPX also currently ships with a transport that uses the excellent
-[`urllib3` library](https://urllib3.readthedocs.io/en/latest/), which can be
-used with the sync `Client`...
+### urllib3 transport
+
+This [public gist](https://gist.github.com/florimondmanca/d56764d78d748eb9f73165da388e546e) provides a transport that uses the excellent [`urllib3` library](https://urllib3.readthedocs.io/en/latest/), and can be used with the sync `Client`...
 
 ```pycon
 >>> import httpx
->>> client = httpx.Client(transport=httpx.URLLib3Transport())
+>>> from urllib3_transport import URLLib3Transport
+>>> client = httpx.Client(transport=URLLib3Transport())
 >>> client.get("https://example.org")
 <Response [200 OK]>
 ```
 
-Note that you'll need to install the `urllib3` package to use `URLLib3Transport`.
+### Writing custom transports
 
 A transport instance must implement the Transport API defined by
 [`httpcore`](https://www.encode.io/httpcore/api/). You
diff --git a/docs/compatibility.md b/docs/compatibility.md
index dbb8941b8b..7b1bc970d8 100644
--- a/docs/compatibility.md
+++ b/docs/compatibility.md
@@ -83,3 +83,11 @@ Besides, `httpx.Request()` does not support the `auth`, `timeout`, `allow_redire
 ## Mocking
 
 If you need to mock HTTPX the same way that test utilities like `responses` and `requests-mock` does for `requests`, see [RESPX](https://github.com/lundberg/respx).
+
+## Networking layer
+
+`requests` defers most of its HTTP networking code to the excellent [`urllib3` library](https://urllib3.readthedocs.io/en/latest/).
+
+On the other hand, HTTPX uses [HTTPCore](https://github.com/encode/httpcore) as its core HTTP networking layer, which is a different project than `urllib3`.
+
+If you're transitioning from Requests to HTTPX, you might want to consider using HTTPX with an [`urllib3` custom transport](/advanced#urllib3-transport) at first, so that `urllib3` is retained as the underlying HTTP networking implementation, allowing you to deal with any differences in behavior as a second phase.
diff --git a/httpx/__init__.py b/httpx/__init__.py
index 3802439466..bfd52806ef 100644
--- a/httpx/__init__.py
+++ b/httpx/__init__.py
@@ -38,7 +38,6 @@
 from ._models import URL, Cookies, Headers, QueryParams, Request, Response
 from ._status_codes import StatusCode, codes
 from ._transports.asgi import ASGITransport
-from ._transports.urllib3 import URLLib3ProxyTransport, URLLib3Transport
 from ._transports.wsgi import WSGITransport
 
 __all__ = [
@@ -101,8 +100,6 @@
     "TransportError",
     "UnsupportedProtocol",
     "URL",
-    "URLLib3ProxyTransport",
-    "URLLib3Transport",
     "WriteError",
     "WriteTimeout",
     "WSGITransport",
diff --git a/httpx/_transports/urllib3.py b/httpx/_transports/urllib3.py
deleted file mode 100644
index c5b7af6cc2..0000000000
--- a/httpx/_transports/urllib3.py
+++ /dev/null
@@ -1,149 +0,0 @@
-import socket
-from typing import Iterator, List, Mapping, Optional, Tuple
-
-import httpcore
-
-from .._config import create_ssl_context
-from .._content_streams import ByteStream, IteratorStream
-from .._exceptions import NetworkError, map_exceptions
-from .._types import CertTypes, VerifyTypes
-
-try:
-    import urllib3
-    from urllib3.exceptions import MaxRetryError, SSLError
-except ImportError:  # pragma: nocover
-    urllib3 = None
-
-
-class URLLib3Transport(httpcore.SyncHTTPTransport):
-    def __init__(
-        self,
-        *,
-        verify: VerifyTypes = True,
-        cert: CertTypes = None,
-        trust_env: bool = None,
-        pool_connections: int = 10,
-        pool_maxsize: int = 10,
-        pool_block: bool = False,
-    ):
-        assert (
-            urllib3 is not None
-        ), "urllib3 must be installed in order to use URLLib3Transport"
-
-        self.pool = urllib3.PoolManager(
-            ssl_context=create_ssl_context(
-                verify=verify, cert=cert, trust_env=trust_env, http2=False
-            ),
-            num_pools=pool_connections,
-            maxsize=pool_maxsize,
-            block=pool_block,
-        )
-
-    def request(
-        self,
-        method: bytes,
-        url: Tuple[bytes, bytes, Optional[int], bytes],
-        headers: List[Tuple[bytes, bytes]] = None,
-        stream: httpcore.SyncByteStream = None,
-        timeout: Mapping[str, Optional[float]] = None,
-    ) -> Tuple[bytes, int, bytes, List[Tuple[bytes, bytes]], httpcore.SyncByteStream]:
-        headers = [] if headers is None else headers
-        stream = ByteStream(b"") if stream is None else stream
-        timeout = {} if timeout is None else timeout
-
-        urllib3_timeout = urllib3.util.Timeout(
-            connect=timeout.get("connect"), read=timeout.get("read")
-        )
-
-        chunked = False
-        content_length = 0
-        for header_key, header_value in headers:
-            header_key = header_key.lower()
-            if header_key == b"transfer-encoding":
-                chunked = header_value == b"chunked"
-            if header_key == b"content-length":
-                content_length = int(header_value.decode("ascii"))
-        body = stream if chunked or content_length else None
-
-        scheme, host, port, path = url
-        default_port = {b"http": 80, "https": 443}.get(scheme)
-        if port is None or port == default_port:
-            url_str = "%s://%s%s" % (
-                scheme.decode("ascii"),
-                host.decode("ascii"),
-                path.decode("ascii"),
-            )
-        else:
-            url_str = "%s://%s:%d%s" % (
-                scheme.decode("ascii"),
-                host.decode("ascii"),
-                port,
-                path.decode("ascii"),
-            )
-
-        with map_exceptions(
-            {
-                MaxRetryError: NetworkError,
-                SSLError: NetworkError,
-                socket.error: NetworkError,
-            }
-        ):
-            conn = self.pool.urlopen(
-                method=method.decode(),
-                url=url_str,
-                headers={
-                    key.decode("ascii"): value.decode("ascii") for key, value in headers
-                },
-                body=body,
-                redirect=False,
-                assert_same_host=False,
-                retries=0,
-                preload_content=False,
-                chunked=chunked,
-                timeout=urllib3_timeout,
-                pool_timeout=timeout.get("pool"),
-            )
-
-        def response_bytes() -> Iterator[bytes]:
-            with map_exceptions({socket.error: NetworkError}):
-                for chunk in conn.stream(4096, decode_content=False):
-                    yield chunk
-
-        status_code = conn.status
-        headers = list(conn.headers.items())
-        response_stream = IteratorStream(
-            iterator=response_bytes(), close_func=conn.release_conn
-        )
-        return (b"HTTP/1.1", status_code, conn.reason, headers, response_stream)
-
-    def close(self) -> None:
-        self.pool.clear()
-
-
-class URLLib3ProxyTransport(URLLib3Transport):
-    def __init__(
-        self,
-        *,
-        proxy_url: str,
-        proxy_headers: dict = None,
-        verify: VerifyTypes = True,
-        cert: CertTypes = None,
-        trust_env: bool = None,
-        pool_connections: int = 10,
-        pool_maxsize: int = 10,
-        pool_block: bool = False,
-    ):
-        assert (
-            urllib3 is not None
-        ), "urllib3 must be installed in order to use URLLib3ProxyTransport"
-
-        self.pool = urllib3.ProxyManager(
-            proxy_url=proxy_url,
-            proxy_headers=proxy_headers,
-            ssl_context=create_ssl_context(
-                verify=verify, cert=cert, trust_env=trust_env, http2=False
-            ),
-            num_pools=pool_connections,
-            maxsize=pool_maxsize,
-            block=pool_block,
-        )
diff --git a/scripts/coverage b/scripts/coverage
index a476e8b278..25a2691074 100755
--- a/scripts/coverage
+++ b/scripts/coverage
@@ -8,4 +8,4 @@ export SOURCE_FILES="httpx tests"
 
 set -x
 
-${PREFIX}coverage report --omit=httpx/_transports/urllib3.py --show-missing --skip-covered --fail-under=100
+${PREFIX}coverage report --show-missing --skip-covered --fail-under=100


Code review comments:
diff_hunk: @@ -83,3 +83,11 @@ Besides, `httpx.Request()` does not support the `auth`, `timeout`, `allow_redire
 ## Mocking
 
 If you need to mock HTTPX the same way that test utilities like `responses` and `requests-mock` does for `requests`, see [RESPX](https://github.com/lundberg/respx).
+
+## Networking layer
+
+`requests` defers most of its HTTP networking code to the excellent [`urllib3` library](https://urllib3.readthedocs.io/en/latest/).
+
+On the other hand, HTTPX uses [HTTPCore](https://github.com/encode/httpcore) as its core HTTP networking layer, which is a different project than `urllib3`.
+
+If you're transitioning from Requests to HTTPX, you might want to consider using HTTPX with an [`urllib3` custom transport](/advanced#urllib3-transport) at first, so that `urllib3` is retained as the underlying HTTP networking implementation, allowing you to deal with any differences in behavior as a second phase.
path: docs/compatibility.md
text: @user1:
I don't think we should be recommending this.

I know it might have been mentioned at some point in the project's past, but it's unlikely to be a sensible route for many folks to follow at this point.