Ensure `import urllib3` can succeed even if trio and twisted aren't installed

[python-trio]

Ensure `import urllib3` can succeed even if trio and twisted aren't installed

[njsmith]

I caught @kennethreitz on freenode #positivepython:

<njs> kennethreitz_: when you have a chance, it'd be good to get your opinion on https://github.com/python-trio/urllib3/issues/25#issuecomment-375850844
<njs> kennethreitz_: since this will eventually probably become part of requests's public API, and someone is working on a patch for it now (https://github.com/python-trio/urllib3/pull/42) :-)
<kennethreitz_> njs: i was going to propose strings myself
<njs> kennethreitz_: I like strings for picking the backend; I'm not as sure about the best way to handle extra arguments
<kennethreitz_> backend = {name="twisted", opt=val} ?
<kennethreitz_> * name:
<njs> kennethreitz_: {"name": "twisted", "opt": val}, I guess, but then Backend("twisted", opt=val) starts to be more appealing :-)
<kennethreitz_> i don't disagree
<njs> but it's true that backend="twisted", backend_args={"opt": val} avoids the extra class, if that's something we care about
<kennethreitz_> i think class is classier :)
<kennethreitz_> like dict would just be shorthand for providing the class
<njs> the extra class ends up becoming part of all downstream libraries too
<kennethreitz_> as long as it can take subclasses, that might be fine
<njs> so I kind of like it better, but nervous about just making that decision for downstream libraries :-)
<kennethreitz_> be bold!
<njs> heh
<njs> basically I figure you're probably the downstream library who's pickiest about the public API, so :-)
<njs> (any objection to my pasting this conversation into the issue?)
<kennethreitz_> nope!
<kennethreitz_> i think requests would likely prefer strings, which get transformed into a class
<kennethreitz_> e.g. they're just shorthand
<kennethreitz_> if you need args, you pass the class
<njs> kennethreitz_: that's what I was thinking too
<njs> kennethreitz_: high five
<kennethreitz_> \o
<njs> o/

So unless anyone else objects, I think let's go with my proposal at the end of this comment, with the duck-typeable Backend class, and a bare string backend="twisted" as equivalent to backend=Backend("twisted")?

[njsmith]

Thinking about it, we probably want to not even try to import trio or twisted until requested (if only because these are non-trivial libraries and importing them takes time, which is not so nice for little scripts that just want to make a few synchronous requests). I think this means that the thing the user passes as their backend= argument should be some sort of placeholder, with the actual backend import somehow delayed until we need to make the first connection.

So what should this placeholder look like? A few options:

# Strings?
AsyncPoolManager(backend="trio")

# Some sort of enum constant?
AsyncPoolManager(backend=TRIO)

# For twisted/asyncio we'll want to be able to specify the reactor/loop...
AsyncPoolManager(backend=Twisted(reactor=myreactor))
AsyncPoolManager(backend="twisted", backend_args=dict(reactor=myreactor))
AsyncPoolManager(backend=Backend("twisted", reactor=myreactor))
AsyncPoolManager(backend=Backend(Backend.TWISTED, reactor=myreactor))

Whatever we choose will probably leak into higher levels of the stack like requests – if requests uses urllib3 under the hood, then requests's users will want to be able to specify any of the supported backends for requests to use, and ideally when urllib3 gets a new backend it shouldn't require code changes to requests. So CC: @kennethreitz

I know @Lukasa will be grumpy about the string version, because pyflakes can't catch if you write backend="troi", tab-completion can't fill it in, etc. (Though I think there's a proposal to allow mypy to check these arguments that must be one of a specific set of values.) It doesn't bother me too much in this case because if you do have a typo then you'll get an error as soon as you create the AsyncPoolManager (which is the same time you'd get the error if you misspelled a constant), but still, proper constants have some value.

A downside to constants though is getting everyone to agree on them. We don't want people to write things like:

requests.AsyncSession(backend=urllib3.something)

because the requests public API should be self-contained, and not expose that it uses urllib3 underneath. In particular, this would break if requests ever switched to using a different underlying http library, and maybe even if it just vendored urllib3 (because then requests._vendored.urllib3.TRIO would be a different object than urllib3.TRIO).

I'm kind of tempted to say that the options are:

# uses the default reactor
AsyncPoolManager(backend="twisted")
# same as above, but more verbose
AsyncPoolManager(backend=Backend("twisted"))
# if you want to specify arguments
AsyncPoolManager(backend=Backend("twisted", reactor=...))

And we can even make the Backend class be something simple and duck-typed, like:

class Backend:
    def __init__(self, backend_name, **kwargs):
        self._backend_name = backend_name
        self._kwargs = kwargs

Requests etc. can re-export this, but this also allows us to skip isinstance(..., Backend) checks (either we have a str or else an object with those attributes), so even if someone does get mixed up and use a Backend object from the wrong vendored copy of urllib3 then everything will still be fine.

[oliland]

I worry that exporting an object would introduce a lot of complexity for the average user (who is likely to be using a wrapper around urllib), so I'm in favour of:

backend="twisted",
backend_args=dict(reactor=...)

As long as we go down in flames when the user misspells their backend (and by extension their backend_args), I think this is OK.

[pquentin]

@kennethreitz Since requests3/requests-core will consume this API, what do you think of the string approach: AsyncPoolManager(backend="twisted", backend_args=dict(reactor=myreactor))?

[pquentin]

Fixed by #47