-
Notifications
You must be signed in to change notification settings - Fork 300
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
Fix pydantic basemodel default input #3013
base: master
Are you sure you want to change the base?
Conversation
Signed-off-by: Future-Outlier <[email protected]>
Signed-off-by: Future-Outlier <[email protected]>
if is_imported("pydantic"): | ||
try: | ||
from pydantic import BaseModel as BaseModelV2 | ||
from pydantic.v1 import BaseModel as BaseModelV1 | ||
|
||
if issubclass(python_type, BaseModelV2): | ||
default_val = default_val.model_dump_json() | ||
elif issubclass(python_type, BaseModelV1): | ||
default_val = default_val.json() | ||
except ImportError: | ||
# Pydantic BaseModel v1 | ||
default_val = default_val.json() |
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 it enough to duck type here?
if hasattr(default_val, "model_dump_json"):
# pydantic v2
default_val = default_val.model_dump_json()
elif hasattr(default_val, "json"):
# pydantic v1
default_val = default_val.json()
else:
encoder = JSONEncoder(python_type)
default_val = encoder.encode(default_val)
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #3013 +/- ##
===========================================
+ Coverage 51.08% 96.54% +45.45%
===========================================
Files 201 11 -190
Lines 21231 723 -20508
Branches 2731 0 -2731
===========================================
- Hits 10846 698 -10148
+ Misses 9787 25 -9762
+ Partials 598 0 -598 ☔ View full report in Codecov by Sentry. |
Tracking issue
flyteorg/flyte#5318
Why are the changes needed?
We should make default input for pydantic basemodel works.
What changes were proposed in this pull request?
add default input logic handling for pydantic basemodel in
to_click_option
.How was this patch tested?
local execution, remote execution, and integration test.
Setup process
Screenshots
local execution
remote execution
Check all the applicable boxes
Related PRs
Docs link