Skip to content

Add warning about unclosed AsyncClient #871

Closed

Description

@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

Activity

  1. florimondmanca commented on Mar 25, 2020

    @florimondmanca
    Contributor

    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?).

  2. kamilglod commented on Mar 25, 2020

    @kamilglod
    Author

    @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:
    Screenshot from 2020-03-25 12-40-08

  3. florimondmanca commented on Mar 25, 2020

    @florimondmanca
    Contributor

    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. :)

  4. Pradhvan commented on Jun 17, 2020

    @Pradhvan

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

  5. Pradhvan commented on Jun 18, 2020

    @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.

  6. added this to the v1.0 milestone on Jul 30, 2020
  7. cdeler commented on Aug 17, 2020

    @cdeler
    Contributor

    @Pradhvan do you mind if I take this?

  8. cdeler commented on Aug 18, 2020

    @cdeler
    Contributor

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

  9. Pradhvan commented on Aug 18, 2020

    @Pradhvan

    @cdeler yes 😄

  10. 8 remaining items

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    concurrencyIssues related to concurrency and usage of async librariesenhancementNew feature or requestgood first issueGood for newcomers

    Type

    No type

    Projects

    No projects

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions