-
-
Notifications
You must be signed in to change notification settings - Fork 32.8k
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
Conversation
Hey there @hunterjm, mind taking a look at this pull request as it has been labeled with an integration ( Code owner commandsCode owners of
|
Hey there @bachya, mind taking a look at this pull request as it has been labeled with an integration ( Code owner commandsCode owners of
|
Hey there @nickw444, @Bre77, mind taking a look at this pull request as it has been labeled with an integration ( Code owner commandsCode owners of
|
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 ❤️) |
Let me chime in here and quickly summarize the situation: 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. |
Just one more note (taken from the readme of my integration):
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 |
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). |
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 |
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. |
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. |
Can we remove the "Draft" for this so it can be reviewed and merged? |
Looks like wheels are ok pydantic-2.10.3-py3-none-any.whl 10-Dec-2024 12:54 456997 |
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. |
Seems ok on production, but I only have a segment of integrations that use pydantic |
There was a problem hiding this 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.
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. |
Home Assistant has migrated to pydantic 2 which is the only reason we kept around v1 support home-assistant/core#131963
Home Assistant has migrated to pydantic 2 which is the only reason we kept around v1 support home-assistant/core#131963
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
Additional information
Checklist
ruff format homeassistant tests
)If user exposed functionality or configuration variables are added/changed:
If the code communicates with devices, web services, or third-party tools:
Updated and included derived files by running:
python3 -m script.hassfest
.requirements_all.txt
.Updated by running
python3 -m script.gen_requirements_all
.To help with the load of incoming pull requests: