Skip to content
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

Upgrade aiohttp to 3.3.0 #14766

Merged
merged 1 commit into from
Jun 7, 2018
Merged

Upgrade aiohttp to 3.3.0 #14766

merged 1 commit into from
Jun 7, 2018

Conversation

fabaff
Copy link
Member

@fabaff fabaff commented Jun 2, 2018

Description:

Changelog: https://github.com/aio-libs/aiohttp/blob/master/CHANGES.rst#330-2018-06-01

Example entry for configuration.yaml (if applicable):

http:

Checklist:

  • The code change is tested and works locally.
  • Local tests pass with tox. Your PR cannot be merged unless tests pass

@fabaff fabaff requested a review from a team as a code owner June 2, 2018 08:03
@homeassistant homeassistant added core small-pr PRs with less than 30 lines. cla-signed labels Jun 2, 2018
@pvizeli
Copy link
Member

pvizeli commented Jun 2, 2018

They changed the timeout handling. Now it is a class and for clients requests, the async_timeouts are strongly discounted while he destroy the internal cleanups.

@balloob, we should cleaning our handling with this PR too...

@fabaff
Copy link
Member Author

fabaff commented Jun 2, 2018

As a reference their discussion: aio-libs/aiohttp#2768

@balloob
Copy link
Member

balloob commented Jun 2, 2018

So we should refractor all async_timeout and use their class instead? I guess easiest will be to just put a default total timeout in our request session and then remove the calls to async timeout?

What will be raised when a timeout occurs, still the exception from asyncio module?

@pvizeli
Copy link
Member

pvizeli commented Jun 6, 2018

@balloob yes. And yes we should use a default timeout object to helpers/aiohttp_client. I think we should also rename aiohttp_client to webclient. (hass.helpers.webclient is better to read)

@balloob
Copy link
Member

balloob commented Jun 7, 2018

So I think that we should write up these issues, but still go ahead and merge this.

@balloob balloob merged commit a6c1192 into dev Jun 7, 2018
@balloob balloob deleted the upgrade-aiohttp branch June 7, 2018 17:49
@ghost ghost removed the small-pr PRs with less than 30 lines. label Jun 7, 2018
@balloob balloob mentioned this pull request Jun 22, 2018
@home-assistant home-assistant locked and limited conversation to collaborators Sep 5, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants