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

Bump pydantic to 2.10.3 and update required deps #131963

Merged
merged 26 commits into from
Dec 10, 2024
Merged

Bump pydantic to 2.10.3 and update required deps #131963

merged 26 commits into from
Dec 10, 2024

Conversation

bdraco
Copy link
Member

@bdraco bdraco commented Nov 30, 2024

Proposed change

fixes #99218

Remaining

changelog:
pydantic: pydantic/pydantic@v1.10.19...v2.10.3

dep changelogs:
aiopurpleair: bachya/aiopurpleair@2022.12.1...2023.12.0
pyaussiebb: yaleman/pyaussiebb@v0.0.15...v0.1.4
xbox-webapi: OpenXbox/xbox-webapi-python@v2.0.11...v2.1.0

blog post: home-assistant/developers.home-assistant#2492

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)
  • Deprecation (breaking change to happen in the future)
  • 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
  • I have followed the perfect PR recommendations
  • The code has been formatted using Ruff (ruff format 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.

To help with the load of incoming pull requests:

@home-assistant
Copy link

Hey there @hunterjm, mind taking a look at this pull request as it has been labeled with an integration (xbox) you are listed as a code owner for? Thanks!

Code owner commands

Code owners of xbox can trigger bot actions by commenting:

  • @home-assistant close Closes the pull request.
  • @home-assistant rename Awesome new title Renames the pull request.
  • @home-assistant reopen Reopen the pull request.
  • @home-assistant unassign xbox Removes the current integration label and assignees on the pull request, add the integration domain after the command.
  • @home-assistant add-label needs-more-information Add a label (needs-more-information, problem in dependency, problem in custom component) to the pull request.
  • @home-assistant remove-label needs-more-information Remove a label (needs-more-information, problem in dependency, problem in custom component) on the pull request.

@home-assistant
Copy link

Hey there @bachya, mind taking a look at this pull request as it has been labeled with an integration (purpleair) you are listed as a code owner for? Thanks!

Code owner commands

Code owners of purpleair can trigger bot actions by commenting:

  • @home-assistant close Closes the pull request.
  • @home-assistant rename Awesome new title Renames the pull request.
  • @home-assistant reopen Reopen the pull request.
  • @home-assistant unassign purpleair Removes the current integration label and assignees on the pull request, add the integration domain after the command.
  • @home-assistant add-label needs-more-information Add a label (needs-more-information, problem in dependency, problem in custom component) to the pull request.
  • @home-assistant remove-label needs-more-information Remove a label (needs-more-information, problem in dependency, problem in custom component) on the pull request.

@home-assistant
Copy link

Hey there @nickw444, @Bre77, mind taking a look at this pull request as it has been labeled with an integration (aussie_broadband) you are listed as a code owner for? Thanks!

Code owner commands

Code owners of aussie_broadband can trigger bot actions by commenting:

  • @home-assistant close Closes the pull request.
  • @home-assistant rename Awesome new title Renames the pull request.
  • @home-assistant reopen Reopen the pull request.
  • @home-assistant unassign aussie_broadband Removes the current integration label and assignees on the pull request, add the integration domain after the command.
  • @home-assistant add-label needs-more-information Add a label (needs-more-information, problem in dependency, problem in custom component) to the pull request.
  • @home-assistant remove-label needs-more-information Remove a label (needs-more-information, problem in dependency, problem in custom component) on the pull request.

@joostlek
Copy link
Member

I'm going to tag @planbnet here as he is known with livisi. At this point we would either fork it ourselves and fix it (but it will add another maintenance burden, so I rather avoid that, but if that is what is needed to get this through, let's go for it). But if @planbnet would be willing to help out here, it could be a way for core to get a better livisi. (I see that your custom integration expanded quite a lot, good to see ❤️)

@planbnet
Copy link
Contributor

Let me chime in here and quickly summarize the situation:
Livisi, the company behind the accessories supported by this integration, went out of business a year ago. Before that, they released a firmware upgrade that changed the SHC (the controller hub) from a cloud-connected device to a local stand-alone device. As part of the same effort, they created this integration (to interface with it in local mode) and submitted a PR to integrate it into Home Assistant. They learned that all interfacing code must be separated into a library and not be part of the integration, so they (badly, in my view) extracted it into the aiolivisi library. There was never any other use for that library apart from being used by this integration.

That’s where I join the story: Livisi asked for help in maintaining the integration, and I wanted to keep my existing devices (just as I was switching from OpenHAB to Home Assistant). So, I offered my help. Unfortunately, I never got access to the library, only to the integration, and shortly afterward, the only maintainer of the library stopped responding. Since enhancing (or fixing anything in) the integration is impossible without changing the library, I created this repository, where I forked both the integration and the library into one repo, cleaned up the code, and started working on new features. This fork is now more mature and supports all base devices that Livisi sold. I don’t think anyone would prefer the official integration over mine.

I’m not particularly interested in maintaining a fork of the library because there is no other use for it than this integration. Unfortunately, this (and probably a lack of extensive unit tests) also prevents replacing the official code with mine. I don’t think that’s really a problem though—there are maybe a few dozen people who use the Livisi stuff with Home Assistant, and the number will only get smaller. I guess the official integration can simply be removed from the official release.

@planbnet
Copy link
Contributor

planbnet commented Nov 30, 2024

Just one more note (taken from the readme of my integration):

Home Assistant requires all communication to the service backend to be handled by a library (as it was done with the original, unmaintained aiolivisi lib). This prevents the merge of this library to the official core code base currently. However, I don't feel it makes sense to move the code to a seperately maintained library as it is only used here and additions often need to happen along the whole code path. Semantically such a split makes sense though, so to make a later separation possible, I added the livisi_ prefix to all the "library" code.

So, if you really want to keep the livisi integration in the core repo and are willing to create a new aiolivisi lib, you can simply take the livisi_ prefixed files from my code, they should not have any references to the home assistant stuff (and don't use pydantic at all).

@joostlek
Copy link
Member

I’m not particularly interested in maintaining a fork of the library because there is no other use for it than this integration.

Don't forget that these libraries can be used for anything people want. By requiring the code to be a library, we as project also give something back to the Python community allowing everyone to use the libraries we use for their own use case outside of Home Assistant. So yes, the initial use case is limited, but in the end, you never know. (There's also more reasoning behind this decision, for example that if we didn't enforce this rule, our codebase would be MASSIVE and not maintainable anymore. So yes, is it extra work? Yes. Does it have benefits to do it this way? Also yes)

Your integration has 184 active installations and the core one has 235. That makes 419. Since not everyone shares analytics, we roughly triple this number to get a real number, that would be about 1300. I would say that is still a substantial amount of users and I would be willing to put effort into helping out and enabling you to get it into core and make it awesome.

Even though livisi is out of business, its awesome to see that you can still control it via HA. I genuinely like the fact that they enabled this and that this can happen and I think if we would remove the integration, we would give the sign that it doesn't work (which is not the case). (Also keep in mind, as project we don't support custom components).

@balloob
Copy link
Member

balloob commented Dec 1, 2024

I agree with @joostlek.

@planbnet would you be willing to publish a fork of your library on PyPI? Then the core integration can be swapped to use the new library and we can move forward with this PR (which is a big hurdle for all integrations).

@planbnet
Copy link
Contributor

planbnet commented Dec 1, 2024

Fine - I took this as an opportunity to learn how to publish a PyPI lib and created https://pypi.org/project/livisi/

This is simply a fork (renamed to just livisi) of the current aiolivisi library – not the improved version of my code. I only backported the removal of pydantic. I retained the versioning scheme of aiolivisi and published v0.0.20 - it should be a drop-in replacement apart from renaming all occurrences of aiolivisi to livisi.

However, I currently don’t have the time to test this with Home Assistant. If someone is willing to test it, please go ahead. Otherwise, I’ll look into this later, but it might take until the holidays.

In the future, I’ll update it to my version (which will completely break the interface, so significant changes to the integration will be required) and bump the version to v1.0.0. Hopefully, this will pave the way for updating the official integration to my unofficial one later as well

@joostlek
Copy link
Member

joostlek commented Dec 1, 2024

That's awesome! ♥️

Feel free to connect with me on Discord so we can chat about it and see where I can help out with that.

@joostlek joostlek mentioned this pull request Dec 1, 2024
19 tasks
@joostlek
Copy link
Member

joostlek commented Dec 1, 2024

#132001

@planbnet
Copy link
Contributor

planbnet commented Dec 1, 2024

Thank you for creating the pr - this made it easy for me to test if the changes still work. After a few more changes it's now correctly parsing the messages without pydantic.

@ardevd
Copy link

ardevd commented Dec 8, 2024

Can we remove the "Draft" for this so it can be reviewed and merged?

@bdraco
Copy link
Member Author

bdraco commented Dec 10, 2024

[pydantic_core-2.27.1-cp312-cp312-musllinux_1_2_..>](https://wheels.home-assistant.io/musllinux/pydantic_core-2.27.1-cp312-cp312-musllinux_1_2_aarch64.whl) 22-Nov-2024 05:40             1960845
[pydantic_core-2.27.1-cp312-cp312-musllinux_1_2_..>](https://wheels.home-assistant.io/musllinux/pydantic_core-2.27.1-cp312-cp312-musllinux_1_2_armv6l.whl) 22-Nov-2024 05:05             2361125
[pydantic_core-2.27.1-cp312-cp312-musllinux_1_2_..>](https://wheels.home-assistant.io/musllinux/pydantic_core-2.27.1-cp312-cp312-musllinux_1_2_armv7l.whl) 22-Nov-2024 05:27             1758678
[pydantic_core-2.27.1-cp312-cp312-musllinux_1_2_..>](https://wheels.home-assistant.io/musllinux/pydantic_core-2.27.1-cp312-cp312-musllinux_1_2_i686.whl) 22-Nov-2024 04:19             2061315
[pydantic_core-2.27.1-cp312-cp312-musllinux_1_2_..>](https://wheels.home-assistant.io/musllinux/pydantic_core-2.27.1-cp312-cp312-musllinux_1_2_x86_64.whl) 22-Nov-2024 04:19             2124916
[pydantic_core-2.27.1-cp313-cp313-musllinux_1_2_..>](https://wheels.home-assistant.io/musllinux/pydantic_core-2.27.1-cp313-cp313-musllinux_1_2_aarch64.whl) 22-Nov-2024 05:36             1960756
[pydantic_core-2.27.1-cp313-cp313-musllinux_1_2_..>](https://wheels.home-assistant.io/musllinux/pydantic_core-2.27.1-cp313-cp313-musllinux_1_2_armv6l.whl) 22-Nov-2024 08:50             2361273
[pydantic_core-2.27.1-cp313-cp313-musllinux_1_2_..>](https://wheels.home-assistant.io/musllinux/pydantic_core-2.27.1-cp313-cp313-musllinux_1_2_armv7l.whl) 22-Nov-2024 05:23             1758457
[pydantic_core-2.27.1-cp313-cp313-musllinux_1_2_..>](https://wheels.home-assistant.io/musllinux/pydantic_core-2.27.1-cp313-cp313-musllinux_1_2_i686.whl) 22-Nov-2024 04:21             2061295
[pydantic_core-2.27.1-cp313-cp313-musllinux_1_2_..>](https://wheels.home-assistant.io/musllinux/pydantic_core-2.27.1-cp313-cp313-musllinux_1_2_x86_64.whl) 22-Nov-2024 04:18             2124775

Looks like wheels are ok

pydantic-2.10.3-py3-none-any.whl 10-Dec-2024 12:54 456997

@bdraco
Copy link
Member Author

bdraco commented Dec 10, 2024

It looks like the v1 shims are not Cython accelerated so its status quo on the performance loss we saw when moving to Python 3.13.

However as soon as the shim is removed in downstream libs the performance should get better.

@bdraco
Copy link
Member Author

bdraco commented Dec 10, 2024

Seems ok on production, but I only have a segment of integrations that use pydantic

@bdraco bdraco marked this pull request as ready for review December 10, 2024 14:09
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.

Mypy changes look good. Happy to revisit as we move downstream libraries off of v1 shims.

@epenet
Copy link
Contributor

epenet commented Dec 10, 2024

Should this be marked as a breaking change, with corresponding section in the PR description?

@frenck frenck merged commit d2303eb into dev Dec 10, 2024
65 checks passed
@frenck frenck deleted the pydantic_2_and_deps branch December 10, 2024 17:27
@bdraco
Copy link
Member Author

bdraco commented Dec 10, 2024

Should this be marked as a breaking change, with corresponding section in the PR description?

It should not be breaking change for users (well hopefully), the target audience is developers, so a development blog post is the right venue for this.

bdraco added a commit to uilibs/uiprotect that referenced this pull request Dec 10, 2024
Home Assistant has migrated to pydantic 2 which
is the only reason we kept around v1 support
home-assistant/core#131963
bdraco added a commit to uilibs/uiprotect that referenced this pull request Dec 11, 2024
Home Assistant has migrated to pydantic 2 which
is the only reason we kept around v1 support
home-assistant/core#131963
@cdce8p cdce8p mentioned this pull request Dec 11, 2024
19 tasks
@github-actions github-actions bot locked and limited conversation to collaborators Dec 11, 2024
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.

Pydantic v2 migration issue