Skip to content

Should BaseTCPStream.is_connection_dropped be async? #356

Closed

Description

@rowillia

Looking at the urllib3 implementation of is_connection_dropped (https://github.com/urllib3/urllib3/blob/f7a4bed04085975918b3469bf94e195df9faf29a/src/urllib3/util/connection.py#L7-L26) it looks like their (pretty robust and battle tested 😄) implementation will sometimes block when testing if a connection is dropped (eventually calls into https://github.com/urllib3/urllib3/blob/f7a4bed04085975918b3469bf94e195df9faf29a/src/urllib3/util/wait.py#L105).

It looks like #346 was closed, deciding the current implementation is good enough for now with some great research, but it sounds like you may want to improve this API in the future in a way that might require blocking. Given you're pre-1.0 it would be good to get the APIs solid on what can eventually be async before 1.0 so you can more easily use a blocking implementation in the future.

I'm new here and just poking around so please LMK if there's any context I'm missing!

Activity

  1. changed the title [-]`BaseTCPStream.is_connection_dropped` should be async[/-] [+]Should `BaseTCPStream.is_connection_dropped` be async?[/+] on Sep 18, 2019
  2. florimondmanca commented on Sep 18, 2019

    @florimondmanca
    Contributor

    Interesting point! The fact that trio.hazmat.wait_readable() is async too seems to support this. :)

    I'm new here and just poking around

    Please do! Questions and suggestions from fresh eyes are invaluable. 🙌

  3. sethmlarson commented on Sep 21, 2019

    @sethmlarson
    Contributor

    Deferring these changes until we need them, thanks @rowillia! ❤️

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

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions