Skip to content

Full Pydantic 2.0 Support + Bug Fixes #691

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

Open
wants to merge 12 commits into
base: main
Choose a base branch
from

Conversation

sethbuff
Copy link
Contributor

When upgrading to pydantic 2.0, we ran into many issues. It seemed like anything outside of a very simple implementation would not work.

We believed initially that we had to remove support for pydantic 1.0 to fix all of these. Now, we see it might be possible to still allow for compatibility across both versions but there are other breaking changes proposed so we just left it as we felt it would be best.

The main issues we initially faced were:

  1. pydantic aliases broke querying
  2. default values were not working when there were multiple levels of inheritance (this was somewhat fixed but we still ran into situations where this would happen. see test: test_model_validate_uses_default_values)
  3. because of the workarounds in place to set default values, model_fields_set was not accurate which resulted in anything that relied on exclude_unset=True to not work
  4. The context object that can be passed in when validating a model was being set to None
  5. KNNExpression was expecting score_field to be a ModelField which was removed in 2.0 and would result in an AttributeError if used

We were also using pydantic in ways that weren't supported and there was a lot of implicit behavior that changed that was not covered in any documentation or the migration guide. That was the core of a lot these issues.

To fix 1, we added field to the redis-om FieldInfo object so we have access to the field name everywhere and we don't have to rely on alias. (candidly, this also made some refactoring on our side much easier)

To fix 2 and 3, we took inspiration from sqlmodel and allowed for explicitly setting when to "index" a model (similar to table=True in sqlmodel). This means that we only set the class attributes to an ExpressionProxy once, which fixed the default errors. This was happening because pydantic was reading the ExpressionProxy set on fields further up in the inheritance tree and thinking it was the default value. This is a breaking change but with how pydantic 2.0 works, we feel it makes the most sense. It also creates an easy way to prevent redis-om models that won't be queried from being indexed.

For 4, we believe requiring pydantic 2.0 (also a breaking change) and using the new field_validator decorator to validate pk fixed this issue.

For 5, we upgraded the KNNExpression to allow for a little more flexibility and updated it to use the redis-om FieldInfo class with name.

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.

2 participants