Skip to content

Allow timeout for AioSession to be ClientTimeout #429

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Mar 8, 2022

Conversation

orsinium
Copy link
Contributor

The PR removes explicit wrapping of the passed timeout into aihttp.ClientTimeout (aiohttp does wrapping on its own if needed), allowing to pass aihttp.ClientTimeout instance as timeout as well as any float. This gives users more control over timeouts.

@orsinium orsinium requested a review from a team February 16, 2022 12:13
@orsinium orsinium requested review from allandialpad and removed request for a team February 16, 2022 12:13
@vidursatija
Copy link

Can we please have this feature? My application would very much enjoy user controlled timeout.

Thank you,
Vidur

Copy link
Member

@TheKevJames TheKevJames left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @orsinium, sorry for delays on reviewing this and thank you for the contribution! Looks like there's a potential issue here around usage in gcloud-rest-auth, but otherwise I love the idea of this changeset! 👍

Copy link
Member

@TheKevJames TheKevJames left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great! Thank you for the PR and for the quick fixes.

@TheKevJames TheKevJames merged commit 0b1a6a4 into talkiq:master Mar 8, 2022
@orsinium orsinium deleted the timeout branch March 9, 2022 07:51
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.

3 participants