-
Notifications
You must be signed in to change notification settings - Fork 106
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
Pydantic should default to v2, or hera should allow it to be configured #984
Comments
Hi @terrykong, thanks for adding the issue. The original PR #795 to add V2 has some history. The problem is specifically that Hera's codebase was written using Pydantic V1, including the generated Argo model classes. This would require a fairly large time investment to provide models for both versions, and increase the maintenance burden for the two codepaths throughout the codebase (@samj1912 explored this initially but decided against it). Long term we do plan to migrate the codebase to V2, but it should not affect end users at this time. I'm curious why you would need to mix Hera models with your own? As Hera builds workflows into yaml I'm not sure what missing feature your own V2 "mixin" models would be providing? Also, the script runner feature lets you use Pydantic V1 or V2 models within your script template functions. |
So in my use case, I was authoring configuration that used pydantic's from hera.workflows.models import Volume
from pydantic import BaseModel
class MyConfig(BaseModel):
foobar: str
volume: Volume
MyConfig.model_validate({"foobar":"a", "volume": {"host_path": {"path": "/mnt"}}})
# Gives
'''
File "/.../venv/lib/python3.10/site-packages/pydantic/main.py", line 509, in model_validate
return cls.__pydantic_validator__.validate_python(
'''
class H(BaseModel):
path: str
class V(BaseModel):
host_path: H
class MyConfigWhichDoesWork(BaseModel):
foobar:str
volume: V
MyConfigWhichDoesWork.model_validate({"foobar":"a", "volume": {"host_path": {"path": "/mnt"}}})
# And this works So basically I want to use pydantic with hera's types but I think the v1 v2 API mismatch is causing pydantic's cryptic error message. |
Thank you @terrykong! Using the Hera classes as part of your configs makes sense 👍 The point remains that it will take a significant effort to allow both v1 and v2 classes to be exported from the library, so I can't give an expected timeframe/exact details for the time being. |
Pre-bug-report checklist
1. This bug can be reproduced using pure Argo YAML
If yes, it is more likely to be an Argo bug unrelated to Hera. Please double check before submitting an issue to Hera.
2. This bug occurs in Hera when...
Bug report
Describe the bug
A clear and concise description of what the bug is:
When mixing in hera models into my own pydantic models, I get errors like this:
Because I'm using the default in pydantic (V2 models), but hera defaults to the v1 models:
I think this is a bug b/c _PYDANTIC_VERSION gets set to 2 even though the imports are v1.
Error log if applicable:
To Reproduce
Full Hera code to reproduce the bug:
Expected behavior
A clear and concise description of what you expected to happen:
I think this should default to import pydantic.v2 first, but given that both versions are supported according to comments in
_pydantic.py
, I think this could also be a feature request in that there should be some variable likeHERA_PYDANTIC_API
that defaults to v2, but lets others choose v1 if they're still stuck on that version.Environment
Additional context
Add any other context about the problem here.
The text was updated successfully, but these errors were encountered: