-
Notifications
You must be signed in to change notification settings - Fork 86
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 support for Pydantic v2 #790
Conversation
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.
Thank you so much, @sanders41 for this PR ❤️
I just have a few questions
|
||
class Task(CamelBase): | ||
uid: str | ||
uid: int |
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.
Is this not breaking?
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.
This is where my stray print
came from with testing this out. It was erroring so I wanted to see the values. uid
gets returned from Meilisearch like: {"uid": 123}
and not {"uid": "123"}
. Pydantic used to convert the int into a string, but now considers this an error since it isn't the correct type.
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.
Indeed the uid is an int
. I was surprised we let it as str
in the code. but I wonder if it will break something
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.
Ahh, you mean with people using it, I didn't consider that. Yes, I suppose it could. I was surprised to see it was a str
initially also.
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.
Yes, that's exactly what I meant, anyway the task_uid is well-defined as an int
so I think it's better to change it.
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.
LGTM! 🎉
bors merge |
This message is sent automatically Thank you very much for submitting your PR! Did you know that throughout the month of June we’re holding a rafle? |
Pull Request
Pydantic v2 was released with several breaking changes. This PR allows users to use Pydantic 2 or Pydantic 1. I went this direction because Pydantic 2 is so new not all packages can use it yet. Because of this I don't think we can drop support for v1 since it would prevent people that still need v1 from using this package.
Related issue
Fixes #789
What does this PR do?
PR checklist
Please check if your PR fulfills the following requirements:
Thank you so much for contributing to Meilisearch!