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 version to ^2.0.0 #191

Merged
merged 2 commits into from
May 19, 2024
Merged

Conversation

mmaeusezahl
Copy link
Contributor

@mmaeusezahl mmaeusezahl commented Mar 16, 2024

The issue #175 also bit me. Luckily bumping the version did not seem too hard.
This PR would solve dependabots #188.

  • The bump-pydantic tool did half of the work (it modified the import statements)
  • It did however not get the root_validator to model_validator change correct
  • I also changed the pydantic json serialization dict to model_dump. This seems work even with the old version, which is however deprecated and generates warning.
  • I added the requirements.txt to the .gitignore file since it seemed to have only been created for the purpose of make requirements.txt.
  • Afterwards I tested if everything still works in my own project (which it did so far).
  • This repository is Apache version 2.0 licensed and by making this PR, I am contributing my changes to the repository under the terms of the Apache 2 license.

Type of changes

  • Add/update a helper script
  • Add/update link to an external resource like a blog post or video
  • Bug fix
  • New feature
  • Test updates
  • Text cleanups/updates

Checklist

  • I have read the CONTRIBUTING document.
  • All new and existing tests pass.
  • Any scripts added use #!/usr/bin/env interpreter instead of potentially platform-specific direct paths (#!/bin/sh is an allowed exception)
  • Scripts added/updated in this PR are all marked executable.
  • Scripts added/updated in this PR do not have a language file extension unless they are meant to be sourced and not run standalone. No one should have to know if a script was written in bash, python, ruby or whatever. Not including file extensions makes it easier to rewrite the script in another language later without having to change every reference to the previous version.
  • I have confirmed that any links added or updated in my PR are valid.

I used the bump-pydantic initially but had to make some additional
changes afterwards. Passes all tests locally so far.

I also inlcuded the requirements.txt in the .gitignore file since
it is only created for the purpose of `make requirements.txt`.

Signed-off-by: Max Mäusezahl <maxmaeusezahl@googlemail.com>
Copy link
Owner

@unixorn unixorn left a comment

Choose a reason for hiding this comment

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

Thank you for testing this. I broke my ankle last October and between physical therapy and various work-related chaos I haven't had time to test the pydantic update.

Signed-off-by: Joe Block <jpb@unixorn.net>
@unixorn unixorn merged commit a3b20d5 into unixorn:main May 19, 2024
3 of 4 checks passed
This was referenced May 19, 2024
@snicker
Copy link

snicker commented Jun 6, 2024

just want to point out that versions of Pydantic > 2.0.0 have added a lot of really wild system level requirements and has made doing things on 32 bit ARM platforms like the RPI zero really difficult- I am having to manually downgrade to a version of ha-mqtt-discoverable to avoid moving to pydantic 2. I know type safety is cool and all, but them moving to a pure C implementation has made relying on it unnecessarily burdensome and adds a ton of bloat to libraries using it.

@snicker
Copy link

snicker commented Jun 6, 2024

to clarify things that I had to do:

  1. pydantic >2 cannot be built on 32bit arm architectures
  2. downgrade from python 3.12 to 3.11 because there is a prebuilt wheel for pydantic on piwheels.com
  3. glibc 2.33 is required for pydantic-core >2, though most rpi distros of debian will not have this- i'm in the midst of building 2.33 to run on this device
  4. I am anticipating # 3 to not work

I use ha-mqtt-discoverable on edge devices that don't need to be powerful or running the latest OS - pydantic core has taken a strange approach in v2 and not added numerous benefits. Understanding your use case here @unixorn - dealing with a conflict - but I'm going to have to pin to ha-mqtt-discoverable to 0.13.1

@mmaeusezahl
Copy link
Contributor Author

Hi,

sorry to hear that my PR lead to such digressions (I got a notification as the author).

Usually I'm all in favor of updating to the most recent dependencies, but for this particular package I understand why broad support across weak/old hardware is useful. I myself might want to target ARM 32bit architectures.

Yet I don't really understand why this issue even appears. Shouldn't pip automatically dismiss ha-mqtt-discoverable 0.13.1 if it can't fulfill the requirements and choose an older version? Like: What specific package did you try to install?

I think that pinning a specific version in a downstream package is the wrong step as it just relocates the problem.

In this case, there might be an ad-hoc, "easy to maintain" solution to support both pydantic v1 and v2 by using the pydantic.v1 module https://docs.pydantic.dev/latest/migration/#continue-using-pydantic-v1-features. I am thinking something like

#### Untested!
try:
   import pydantic.v1 as pydantic_v1
except:
   import pydantic as pydantic_v1

I did purposely not choose this path (using the new API instead of the the pydantic.v1 compatibility layer) in my PR since I wanted to prevent accumulation of technical debt. But I guess under these specific circumstance I might be a temporary fix for ha-mqtt-discoverable to allow both pydantic v1 and pydantic v2.

Imo, in the long run this should be treated differently. I imagine:

  • Raising this as an upstream issues at pydantic (i.e. asking for the provision of ARM 32bit wheels)
  • Switching to a different schema validator (maybe https://github.com/alecthomas/voluptuous as it is used by HASS afaik)

Yet both will probably require major work.

@unixorn
Copy link
Owner

unixorn commented Jun 7, 2024

I like the idea of using the same validator as HASS. I don't really have time rn to work on this, but I'm happy to accept more PRs.

I'll add the 0.13.1 pin to the readme.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants