Add warning about unclosed AsyncClient

[kamilglod]

It would be nice to have an info about some unclosed AsyncClient() instance. Here is an example how it is implemented in aiohttp: https://github.com/aio-libs/aiohttp/blob/693ed0cce0c67646f8df18c93e448d10782c2811/aiohttp/client.py#L268-L286

[florimondmanca]

Hi @kamilglod

Can you share what the UX for this in aiohttp looks like? (eg maybe a snippet of an interactive interpreter session…)

I'm just wondering if this would actually provide value, or just add noise when shutting down an interpreter (which should force-close any dangling resources anyway, no?).

[kamilglod]

@florimondmanca closing all connections sounds fine but question is whether it is technically doable as close() is an async function and we cannot await it inside __dell__(). Additionally I think that such an information is very helpful in tracking buggy code with unclosed resources
Here is an example output from aiohttp:

[florimondmanca]

Thanks, the REPL snippet is useful.

An implementation similar to aiohttp is technically possible — ie log a warning but don’t attempt to close the client (notably because, as you said, it’s an async operation).

One bit to keep in mind is that this would need to be async library agnostic (asyncio/trio) - so the check needs to be deferred to the async backends at some point - so this doesn’t seem like a trivial addition. But if you’d like to give it a shot feel free to. :)

[Pradhvan]

@florimondmanca If anyone is not working on it. I would like to give it a try. 😄

[Pradhvan]

I was setting up the project locally and in the step of installing dependencies. Should it explicitly mention that scripts/install will automatically create a virtual environment?

You can now install the project and its dependencies using:
$ cd httpx
$ scripts/install

Or is it just me that went ahead creating a virtual env and running the command scripts/install hoping it would install dependencies in the environment I created.

[cdeler]

@Pradhvan do you mind if I take this?

[cdeler]

@florimondmanca should we add the similar check for httpx.Client?

[Pradhvan]

@cdeler yes 😄