Skip to content

Update to Pydantic V2 #588

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

Merged
merged 10 commits into from
Jul 23, 2024
Merged

Conversation

gatesn
Copy link
Contributor

@gatesn gatesn commented Jul 18, 2024

Summary

Updates betterproto to generate data classes for Pydantic V2.

The issues I ran into were:

  • root_validator is deprecated
    • Replaced by model_validate(mode='after'), required updating the field validator to use getattr on the built model
  • betterproto.Enum was not recognized by Pydantic as an enum type.
    • I don't have the history for why this project doesn't use IntEnum, but I worked around this by creating a Pydantic serializer that will validate integers (it does not enforce a max integer to allow for new enum variants to be added).
    • An alternate solution here would just be to include the __get_pydantic_core_schema__ function on every generated Enum class. Open to switching to this.
  • Pydantic no longer raises ValueError when constructing a data class with a missing argument.
    • I think this is ok since it now better matches the builtin behavior, I changed the test to allow this.
    • I added a test demonstrating that Pydantic will validate the correct type.

Fixes #530

Checklist

  • If code changes were made then they have been tested.
    • I have updated the documentation to reflect the changes.
  • This PR fixes an issue.
  • This PR adds something new (e.g. new method or parameters).
    • This change has an associated test.
  • This PR is a breaking change (e.g. methods or parameters removed/renamed)
  • This PR is not a code change (e.g. documentation, README, ...)

@gatesn
Copy link
Contributor Author

gatesn commented Jul 18, 2024

I inlined the Pydantic serializer into every generated enum since typing.Annotated only exists in Python >=3.9

@Gobot1234
Copy link
Collaborator

Fwiw if the code is easier to generate/understand use typing_extensions.Annotated. It is already a dep of the project and probably pydantic

@gatesn
Copy link
Contributor Author

gatesn commented Jul 18, 2024

I don't mind either way, your call

@gatesn
Copy link
Contributor Author

gatesn commented Jul 18, 2024

I just discovered the poe format task 🤦

@Gobot1234
Copy link
Collaborator

I don't mind either way, your call

code looks fine to me as is :)

@cetanu
Copy link
Collaborator

cetanu commented Jul 19, 2024

relock and the checks should go green

@gatesn
Copy link
Contributor Author

gatesn commented Jul 19, 2024

Should be fixed

@gatesn
Copy link
Contributor Author

gatesn commented Jul 23, 2024

@Gobot1234 I've resolved conflicts against develop. Is this g2g now?

@Gobot1234
Copy link
Collaborator

CI looks good :)

@gatesn
Copy link
Contributor Author

gatesn commented Jul 23, 2024

Reverted the import change


{% endfor %}
{% endif %}
{% for message in output_file.messages %}
@dataclass(eq=False, repr=False)
@dataclasses.dataclass(eq=False, repr=False)
Copy link
Collaborator

Choose a reason for hiding this comment

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

This needs reverting

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yup, just caught that, apologies

@gatesn
Copy link
Contributor Author

gatesn commented Jul 23, 2024

@Gobot1234 are we all set? Keen to start getting some model validation in my protos 😄

@Gobot1234
Copy link
Collaborator

Yep, thanks for this!

@Gobot1234 Gobot1234 merged commit 5fdd0bb into danielgtaylor:master Jul 23, 2024
19 checks passed
@gatesn
Copy link
Contributor Author

gatesn commented Aug 5, 2024

@Gobot1234 would you be open to publishing a v2.0.0-beta7?

@gatesn gatesn deleted the ngates/pydanticv2 branch August 5, 2024 17:03
@Gobot1234
Copy link
Collaborator

If you can write up the changelog I'm happy to merge it (I don't have perms to merge without review) and publish.

@gatesn
Copy link
Contributor Author

gatesn commented Aug 5, 2024

Actually it looks like there’s some weird behaviour with the PLACEHOLDER object. Should that be replaced with default primitive value for the given type? Not sure…

@@ -1,9 +1,16 @@
# Generated by the protocol buffer compiler. DO NOT EDIT!
# sources: google/protobuf/any.proto, google/protobuf/api.proto, google/protobuf/descriptor.proto, google/protobuf/duration.proto, google/protobuf/empty.proto, google/protobuf/field_mask.proto, google/protobuf/source_context.proto, google/protobuf/struct.proto, google/protobuf/timestamp.proto, google/protobuf/type.proto, google/protobuf/wrappers.proto
# plugin: python-betterproto

# This file has been @generated
Copy link
Contributor

Choose a reason for hiding this comment

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

@Gobot1234 I thought you specifically said you didn't want this @generated label on these files during this PR: #568.

I'm opening a PR for a CHANGELOG and bump in version, and wondering if you think this is blocking.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Ah yep good catch, would you mind removing it in your PR so it's easier to merge?

@JitPackJoyride JitPackJoyride mentioned this pull request Aug 11, 2024
7 tasks
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.

pydantic v2 support
4 participants