Ensure proxying to Unix sockets constructs separate HTTP clients

[takluyver]

I started to experiment with serving things via Unix sockets (#337), but when I have two separate proxied services, I could only reach one via Jupyterhub; whichever one I used first would also get the requests meant for the other. This change fixes that.

This seems to be because Tornado's HTTP client objects are singletons by default - the docs say:

instances are reused as a kind of pseudo-singleton (one per IOLoop). The keyword argument force_instance=True can be used to suppress this singleton behavior. Unless force_instance=True is used, no arguments should be passed to the AsyncHTTPClient constructor.

I think this bug would also affect proxying to a mixture of Unix sockets and TCP sockets, though I haven't tried. If you access something backed by a Unix socket first, that will get any later requests for other proxies; if you use a TCP socket first, anything on a Unix socket will be inaccessible. This should all be fixed by this change - TCP sockets will keep using the singleton HTTP client, and Unix sockets will get their own client each time.

I considered caching the HTTP client objects for Unix sockets in some other way, but it's extra complexity, and I think setting up a SimpleAsyncHTTPClient is cheap enough that we can just do it each time. 🤞

[takluyver]

The acceptance test failures are unrelated to this change - I've opened #416 to fix the tests.

[manics]

Does this result in one client per proxy backend, or a new client for everyone HTTP call from the browser?

[takluyver]

Does this result in one client per proxy backend, or a new client for everyone HTTP call from the browser?

For backends using Unix sockets, this creates a new client for each HTTP request. I've gone with this simple option for now because it doesn't look like creating a client does anything very expensive. Here's what I think is the relevant code in Tornado: SimpleAsyncHTTPClient.initialize() and AsyncHTTPClient.initialize(). But if this turns out to be a problem, we could store one client object per proxy handler.

For proxying to TCP, it will reuse a single HTTP client across all backends, as before.

[manics]

Thanks! Since the unix socket support is quite new I think we can accept a potential inefficiency unless someone reports otherwise.

[takluyver]

Thanks! And sorry I only realised this after the release.

I know this can be an annoying topic for maintainers, but do you have a timeline in mind for the next release? 🙂