Skip to content

Check disconnections on connection reacquiry - #145

Merged
lovelydinosaur merged 8 commits into
masterfrom
check-disconnections-on-connection-reacquiry
Jul 25, 2019
Merged

lovelydinosaur merged 8 commits into
masterfrom
check-disconnections-on-connection-reacquiry

Conversation

@lovelydinosaur

Copy link
Copy Markdown
Member

Check for connection aliveness on re-acquiry from connection pool.

Superseeds #143

Great work on this @yeraydiazdiaz! There was a last little bit of extra cleanup on the connection pool handling that we can do now, which it made sense for me to pick up. I think we're good to go on this one now.

@lovelydinosaur

Copy link
Copy Markdown
Member Author

I've pulled in a last little bit of extra work on top of this in #145 which I think is now good to go.

One particularly interesting point from @njsmith, here #143 (comment)

There's StreamReader.at_eof, but that will fail if the HTTP server is polite and sends a 408 Request Timeout before it closes the connection.

Okay, that's an interesting case. 馃槕

I'd suggest we start by getting this change in as it stands. We can the confirm if we need to finesse the specific is_connection_dropped() implementation details as a separate improvement on top of this.

@sethmlarson

Copy link
Copy Markdown
Contributor

Should we open a separate issue to handle the 408 case?

@lovelydinosaur

Copy link
Copy Markdown
Member Author

Yeah, defo. On it.

@lovelydinosaur
lovelydinosaur merged commit ec365c0 into master Jul 25, 2019
@lovelydinosaur
lovelydinosaur deleted the check-disconnections-on-connection-reacquiry branch July 25, 2019 21:52
@lovelydinosaur

Copy link
Copy Markdown
Member Author

Released as 0.6.8

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

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants