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

Add Python 3.10 to CI #59729

Merged
merged 19 commits into from
Feb 17, 2022
Merged

Add Python 3.10 to CI #59729

merged 19 commits into from
Feb 17, 2022

Conversation

frenck
Copy link
Member

@frenck frenck commented Nov 15, 2021

Breaking change

The following integrations have been disabled because they have incompatible Python dependencies for Home Assistant.

  • apcupd
  • apns
  • xbee

Proposed change

Adds Python 3.10 to the CI

Notes:

  • zwave has now commented dependencies, and tests are disabled. This allows us to ship it with Python 3.9, but not block Python 3.10
  • icloud tests have been disabled, as those are currently not yet 3.10 compatible. Can be enabled again once we bump pyicloud.

Problematic packages:

  • pyhton-nest -> nest integration.
    Update for python3.10 moving Callable usage to collections.abc jkoelker/python-nest#174
    'collections' has no attribute 'Callable'
    nest/nest.py:159
    
  • velbusaio -> velbus integration.
    Python 3.10 incompatible cereal2nd/velbus-aio#29
       File "/__w/core/core/venv/lib/python3.10/site-packages/velbusaio/protocol.py", line 51, in __init__
      self._send_queue = asyncio.Queue(loop=self._loop)
      TypeError: As of 3.10, the *loop* parameter was removed from Queue() since it is no longer necessary
    
  • keyring used by pyicloud -> icloud integration.
    Python 3.10 incompatible picklepete/pyicloud#368
    venv/lib/python3.10/site-packages/keyring/util/properties.py:1: in <module>
        from collections import Callable
    E   ImportError: cannot import name 'Callable' from 'collections'
    
  • samsungtv integration. ==> Remove passing loop into sleep in SamsungTV #66030
    homeassistant/components/samsungtv/media_player.py:248: in async_play_media
      await asyncio.sleep(KEY_PRESS_TIMEOUT, self.hass.loop)
    E       TypeError: sleep() got an unexpected keyword argument 'loop'
    tests/components/samsungtv/test_media_player.py:718: TypeError
    
  • azure -> azure_event_hub integration ==> Azure Event Hub incompatible with Python 3.10 #66032
      File "/__w/core/core/homeassistant/components/azure_event_hub/client.py", line 26, in test_connection
      async with self.client as client:
    File "/__w/core/core/homeassistant/components/azure_event_hub/client.py", line 64, in client
      return EventHubProducerClient(
    File "/__w/core/core/venv/lib/python3.10/site-packages/azure/eventhub/aio/_producer_client_async.py", line 91, in __init__
      ALL_PARTITIONS: self._create_producer()
    File "/__w/core/core/venv/lib/python3.10/site-packages/azure/eventhub/aio/_producer_client_async.py", line 159, in _create_producer
      handler = EventHubProducer(
    File "/__w/core/core/venv/lib/python3.10/site-packages/azure/eventhub/aio/_producer_async.py", line 94, in __init__
      self._lock = asyncio.Lock(loop=self._loop)
    File "/usr/local/lib/python3.10/asyncio/locks.py", line 77, in __init__
      super().__init__(loop=loop)
    File "/usr/local/lib/python3.10/asyncio/mixins.py", line 17, in __init__
      raise TypeError(
    TypeError: As of 3.10, the *loop* parameter was removed from Lock() since it is no longer necessary
    

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 15, 2021
@frenck frenck mentioned this pull request Nov 16, 2021
43 tasks
@frenck frenck mentioned this pull request Dec 2, 2021
22 tasks
@frenck frenck force-pushed the frenck-2021-3009 branch 2 times, most recently from 97db253 to 9c26e95 Compare December 2, 2021 22:21
@frenck frenck force-pushed the frenck-2021-3009 branch 2 times, most recently from 748c2eb to 0845bab Compare January 11, 2022 12:55
@cdce8p
Copy link
Member

cdce8p commented Jan 11, 2022

I've spend some time today looking into it. Some things I noticed which could be helpful in getting this PR done.

  • Due to a bug in the vendored packaging lib, pip versions prior to 20.3.2 won't install wheels from PyPI even if they have been uploaded. Update packaging version to install Python 3.10 wheels. pypa/pip#9272
    The main issue for us is that 20.3 introduced the new dependency resolver which doesn't yet work for us. For < 21.0, e.g 20.3.4, it's possible to add --use-deprecated=legacy-resolver to pip install to still use the old resolver. The other option would obviously be fixing Upgrade to newer Python pip>=21.0 #59769 but that might take some time.
  • Pandas started uploading wheels for all major platforms only with >= 1.3.4 (for Python 3.10). Looking at the changelog, both 1.3.4 and 1.3.5 should be compatible with 1.3.0.
  • We currently pin grpcio==1.31.0 which seems to be incompatible with Python 3.10. Just looking at PyPI, version 1.41.0 seems to be the first for which the maintainers uploaded wheels.
  • homeassistant-pyozw is incompatible, too. I haven't looked at it that closely, but from what I've seen it used cython. Thus it might be possible to rebuild it with a new cython version without much effort. Then again, the integration is officially deprecated, not sure it's worth it anymore.
  • I initially had some issues building both numpy and pandas with Github actions. Installing software-properties-common in addition to cmake helped.

If you're only interested in the prepare-tests step for Python 3.10, you could use some temporary commands to speed up testing.

  • Adding continue-on-error: true to the Create full Python ${{ matrix.python-version }} virtual environment step will store the wheel cache. Just make sure to disable the Restore full Python ${{ matrix.python-version }} virtual environment step, otherwise it won't do anything. As the prepare-tests will never fail, I would also suggest adding if: false to the pytest job temporarily.

Hope these help 🤞🏻

@frenck
Copy link
Member Author

frenck commented Jan 11, 2022

Pandas started uploading wheels for all major platforms only with >= 1.3.4 (for Python 3.10). Looking at the changelog, both 1.3.4 and 1.3.5 should be compatible with 1.3.0.

The pandas constrain was here because of Alpine 3.13, which we no longer have. The constrain has been lifted in this PR.

We currently pin grpcio==1.31.0 which seems to be incompatible with Python 3.10. Just looking at PyPI, version 1.41.0 seems to be the first for which the maintainers uploaded wheels.

Currently in progress. As a matter of fact, we have released a new wheels builder today that should deal with the last bits of issues we had. We are currently at our third attempt to update to 1.43.0 (thanks to @allenporter for that effort).

Ref: #63911

homeassistant-pyozw is incompatible, too. I haven't looked at it that closely, but from what I've seen it used cython. Thus it might be possible to rebuild it with a new cython version without much effort. Then again, the integration is officially deprecated, not sure it's worth it anymore.

The integration will be removed. This has been discussed already.

I initially had some issues building both numpy and pandas with Github actions. Installing software-properties-common in addition to cmake helped.

Not an issue here.

If you're only interested in the prepare-tests step for Python 3.10, you could use some temporary commands to speed up testing.

Not in a hurry. This will not be merged this release anyways, so we have over 1,5 months left ish to make it in time for 2022.3 (that is not a hard deadline though).

@frenck
Copy link
Member Author

frenck commented Jan 11, 2022

The other option would obviously be fixing Upgrade to newer Python pip #59769 but that might take some time.

PS: I've "resolved" that one locally (Well threw out anything that would cause an issue, just to get somewhere workable), however, it takes pip over 12 hours to resolve our full dependency tree (on an AMD Ryzen 7 machine). Which is not a workable solution.

@cdce8p
Copy link
Member

cdce8p commented Jan 11, 2022

The other option would obviously be fixing Upgrade to newer Python pip #59769 but that might take some time.

PS: I've resolved that one locally, however, it takes pip over 12 hours to resolve our full dependency tree (on an AMD Ryzen 7 machine). Which is not a workable solution.

😮 Ok, yeah 12h is indeed too long. Maybe a naive idea, what about this? (Assuming we start from a no-conflicts state.)
Dependency resolution likely only takes this long as pip constantly backtracks and installs multiple different versions in the process. It should be possible to speed this up by pinning every dependency from the start . I.e. use the output of pip freeze as constraints file.

Let's next say we want to update a "normal" dependency. Pip should be able to check fairly quickly if there are any conflicts with the given constraints. If there are, the next step would be to loosen only the conflicting dependency in the requirements file and try again to see if pip can find a solution now. Afterwards update the constraints.

At first this is a much more manual process, but maybe steps could be automated as part of the CI pipeline.

@cdce8p
Copy link
Member

cdce8p commented Jan 11, 2022

Something like pip-compile with its --upgrade-package option could also work. Although you might have already tried that one. https://pip-tools.readthedocs.io/en/latest/

@frenck
Copy link
Member Author

frenck commented Jan 11, 2022

Something like pip-compile with its --upgrade-package option could also work.

The fact we even need to consider those tools is just too stupid to put into words.

@frenck frenck force-pushed the frenck-2021-3009 branch 2 times, most recently from 2b79019 to 480d283 Compare February 7, 2022 14:16
@cdce8p
Copy link
Member

cdce8p commented Feb 7, 2022

For hyperframe:
The requirement probably needs to be >=6.0. Version 5.2 still uses the removed import.
https://github.com/python-hyper/hyperframe/blob/v5.2.0/hyperframe/flags.py#L10-L14

Regarding the apns tests:
If you don't want to remove them just yet, you could skip the file for pytest.
https://docs.pytest.org/en/latest/how-to/skipping.html#summary

@frenck
Copy link
Member Author

frenck commented Feb 7, 2022

Regarding the apns tests

The integration is pending deletion and has 0 active users according to statistics. I think we can safely remove it 😉

The requirement probably needs to be >=6.0

We can't, as we need <6.0. Additionally, 5.2.0 imports it conditionally, works :)

@cdce8p
Copy link
Member

cdce8p commented Feb 7, 2022

Regarding the apns tests

The integration is pending deletion and has 0 active users according to statistics. I think we can safely remove it 😉

👍🏻

The requirement probably needs to be >=6.0

We can't, as we need <6.0. Additionally, 5.2.0 imports it conditionally, works :)

Got confused with the line number there. The invalid use was in L14, same as the conditional import.

@frenck frenck marked this pull request as ready for review February 17, 2022 10:19
@frenck frenck requested review from a team and Quentame as code owners February 17, 2022 10:19
@frenck frenck merged commit 276fd4f into dev Feb 17, 2022
@frenck frenck deleted the frenck-2021-3009 branch February 17, 2022 13:58
@github-actions github-actions bot locked and limited conversation to collaborators Feb 18, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
breaking-change cla-signed small-pr PRs with less than 30 lines.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants