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.

Sign up for GitHub

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

Jump to bottom

Bump to aiohttp 3.8.0 #58974

Merged
merged 19 commits into from
Nov 4, 2021
Merged

Bump to aiohttp 3.8.0 #58974

merged 19 commits into from
Nov 4, 2021

Conversation

bdraco
Copy link
Member

@bdraco bdraco commented Nov 3, 2021 •
edited
Loading

Breaking change

Bump to aiohttp to 3.8.0

This version of aiohttp requires async_timeout 4.0.0 which has multiple breaking changes:

  • async_timeout.timeout drops the loop= keyword arg.
  • async_timeout.current_task has been removed.
  • The timeout.remaining property has been removed. (No backward compatibility is provided as all instances in core have been changed)
  • The timeout.timeout property has been removed. (No backward compatibility is provided as no usage of this was found)

To facilitate the transition, temporary compatibility code has been added. The backwards compatibility will be removed in 2022.2:

  • async_timeout.timeout is replaced with a backwards compatible transitional version that logs a warning when callers use the loop= argument.
  • async_timeout.current_task is replaced with a backwards compatible transitional version that warns asyncio.current_task should be used instead.

Changes: https://github.com/aio-libs/aiohttp/releases/tag/v3.8.0 https://github.com/aio-libs/async-timeout/releases/tag/v4.0.0

Type of change

  • Dependency upgrade
  • Bugfix (non-breaking change which fixes an issue)
  • New integration (thank you!)
  • New feature (which adds functionality to an existing integration)
  • Breaking change (fix/feature causing existing functionality to break)
  • Code quality improvements to existing code or addition of tests

Additional information

  • This PR fixes or closes issue: fixes #
  • This PR is related to issue:
  • Link to documentation pull request:

Checklist

  • The code change is tested and works locally.
  • Local tests pass. Your PR cannot be merged unless tests pass
  • There is no commented out code in this PR.
  • I have followed the development checklist
  • The code has been formatted using Black (black --fast homeassistant tests)
  • Tests have been added to verify that the new code works.

If user exposed functionality or configuration variables are added/changed:

If the code communicates with devices, web services, or third-party tools:

  • The manifest file has all fields filled out correctly.
    Updated and included derived files by running: python3 -m script.hassfest.
  • New or updated dependencies have been added to requirements_all.txt.
    Updated by running python3 -m script.gen_requirements_all.
  • For the updated dependencies - a link to the changelog, or at minimum a diff between library versions is added to the PR description.
  • Untested files have been added to .coveragerc.

The integration reached or maintains the following Integration Quality Scale:

  • No score or internal
  • 🥈 Silver
  • 🥇 Gold
  • 🏆 Platinum

To help with the load of incoming pull requests:

@probot-home-assistant probot-home-assistant bot added the small-pr PRs with less than 30 lines. label Nov 3, 2021
@hmmbob hmmbob mentioned this pull request Nov 3, 2021
Closed
@bdraco
Copy link
Member Author

bdraco commented Nov 3, 2021

Upgraded a few production installs and everything seems ok.

There are likely going to be side effects that are unseen so good to get it in at the top of the release cycle

@bdraco bdraco marked this pull request as ready for review November 3, 2021 17:30
pvizeli
pvizeli reviewed Nov 3, 2021
View reviewed changes
homeassistant/async_timeout_backcompat.py
@@ -0,0 +1,67 @@
"""Provide backwards compat for async_timeout."""
Copy link
Member

@pvizeli pvizeli Nov 3, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

maybe more fit for living inside utils?

Copy link
Member Author

@bdraco bdraco Nov 3, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

I tried to mirror how block_async_io works with the thought it would be easy to delete the file once the time window had passed.

https://github.com/home-assistant/core/pull/58974/files#diff-79667bda301cc51e7bdf9df436c097570bd3ea3bf7a78091029a8fef9df27587R85

I can move it if utils is a better place.

Copy link
Member

Choose a reason for hiding this comment

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

I think it's fine here. It will be removed anyway.

balloob
balloob approved these changes Nov 3, 2021
View reviewed changes
Copy link
Member

@balloob balloob left a comment

Choose a reason for hiding this comment

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

Let's verify it works with ingress and then this is good to go 👍

Kane610
Kane610 approved these changes Nov 3, 2021
View reviewed changes
Copy link
Member

@Kane610 Kane610 left a comment

Choose a reason for hiding this comment

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

Looks Ok for Axis, deCONZ and UniFi

allenporter
allenporter approved these changes Nov 4, 2021
View reviewed changes
Copy link
Contributor

@allenporter allenporter left a comment

Choose a reason for hiding this comment

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

LG for nest

ludeeus
ludeeus approved these changes Nov 4, 2021
View reviewed changes
Copy link
Member

@ludeeus ludeeus left a comment

Choose a reason for hiding this comment

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

Tested a handful of add-ons with ingress and no apparent issue was shown while using them 👍

@bdraco
Copy link
Member Author

bdraco commented Nov 4, 2021 •
edited
Loading

Thanks for testing.

Let's merge this so devs have the most amount of warning possible to update deps that need to have the loop= argument removed.

@bdraco bdraco merged commit 10d6247 into home-assistant:dev Nov 4, 2021
@bdraco bdraco deleted the aiohttp_380 branch November 4, 2021 15:07
@bdraco
Copy link
Member Author

bdraco commented Nov 4, 2021

Discovered async_timeout.timeout changes needed:

home-assistant-ecosystem/python-volkszaehler#7
timmo001/system-bridge-connector-py#62
aequitas/python-rflink#61
Klathmon/python-sochain-api#3
Klathmon/python-family-hub-local#2
ludeeus/pytautulli#23
ludeeus/pylaunches#3
ludeeus/pyhaversion#65
ufodone/pyenvisalink#15
mezz64/pyEmby#10
bollewolle/pydelijn#11
https://github.com/coreGreenberet/homematicip-rest-api/pull/424
home-assistant-ecosystem/python-opensensemap-api#2
home-assistant-ecosystem/python-luftdaten#10
kstaniek/python-ampio-smog-api#3
timmo001/aiolyric#18
home-assistant-ecosystem/python-hole#10
home-assistant-ecosystem/python-glances-api#3
hacs/integration#2260
cytech/Home-Assistant-wundergroundpws#75
sander76/aio-powerview-api#13
ludeeus/pytraccar#10

@bdraco
Copy link
Member Author

bdraco commented Nov 4, 2021

Discovered async_timeout.current_task changes needed:

Swamp-Ig/pizone#15

natekspencer pushed a commit to natekspencer/home-assistant-core that referenced this pull request Nov 5, 2021
@bdraco @natekspencer
Bump to aiohttp 3.8.0 (home-assistant#58974)
c07dd5f
@ludeeus ludeeus mentioned this pull request Nov 5, 2021
Merged
22 tasks
@github-actions github-actions bot locked and limited conversation to collaborators Nov 5, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@pvizeli pvizeli pvizeli left review comments

@azogue azogue azogue left review comments

@Kane610 Kane610 Kane610 approved these changes

@ludeeus ludeeus ludeeus approved these changes

@allenporter allenporter allenporter approved these changes

@MartinHjelmare MartinHjelmare MartinHjelmare approved these changes

@balloob balloob balloob approved these changes

@andrey-git andrey-git Awaiting requested review from andrey-git andrey-git is a code owner

@bieniu bieniu Awaiting requested review from bieniu bieniu is a code owner

@cgtobi cgtobi Awaiting requested review from cgtobi

@dgomes dgomes Awaiting requested review from dgomes dgomes is a code owner

@elupus elupus Awaiting requested review from elupus elupus is a code owner

@fabaff fabaff Awaiting requested review from fabaff fabaff is a code owner

@frenck frenck Awaiting requested review from frenck

@janiversen janiversen Awaiting requested review from janiversen

@jjlawren jjlawren Awaiting requested review from jjlawren jjlawren is a code owner

@michaelarnauts michaelarnauts Awaiting requested review from michaelarnauts

@ochlocracy ochlocracy Awaiting requested review from ochlocracy ochlocracy is a code owner

@robbiet480 robbiet480 Awaiting requested review from robbiet480

Assignees
No one assigned
Labels
breaking-change cla-signed small-pr PRs with less than 30 lines.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
9 participants
@bdraco @balloob @MartinHjelmare @allenporter @azogue @ludeeus @pvizeli @Kane610 @homeassistant