-
-
Notifications
You must be signed in to change notification settings - Fork 387
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 v2 pydantic_model_creator nullable field not optional #1465
Fix pydantic v2 pydantic_model_creator nullable field not optional #1465
Conversation
Pull Request Test Coverage Report for Build 6497193654
💛 - Coveralls |
This PR solve a lot of things... I think it should be taken as hotfix @long2ice . |
Thanks! Since 0.20.0 was released, so changelog should be in new section 0.20.1 |
I was reviewing @waketzheng 's code. And, I humbly submit that in order to set as nullable tortoise-orm/tortoise/contrib/pydantic/creator.py Lines 352 to 365 in ba593c3
Should be set to: if (
field_type is relational.ForeignKeyFieldInstance
or field_type is relational.OneToOneFieldInstance
or field_type is relational.BackwardOneToOneRelation
):
is_to_one_relation = True
model = get_submodel(fdesc["python_type"])
if model:
if fdesc.get("nullable"):
json_schema_extra["nullable"] = True
# set default_factory to None in the field config
fconfig["default_factory"] = lambda: None
if fdesc.get("nullable") or field_default is not None:
model = Optional[model] # type: ignore
# create property with that config
properties[fname] = (model, Field(**fconfig)) |
@plusiv After set default_factory to None in the field config, unittest failed at tests/contrib/test_pydantic.py |
Hi again @waketzheng ! I've cloned your changes and tested with the changes that I suggested before, and figure out that the reason is because the hardcoded schema used in the test To explain my point, let's take a look to the models tortoise-orm/tests/testmodels.py Lines 74 to 113 in c030c9c
Since the field tortoise-orm/tests/contrib/test_pydantic.py Lines 191 to 198 in c030c9c
But, with the nullable tortoise-orm/tests/contrib/test_pydantic.py Lines 84 to 99 in c030c9c
Notice that the element I think that re-generating the hardcoded schema with the changes that I suggested before and replacing the old ones with the new ones, should make the test run without problems. I could help with this if you like. |
Hello again @waketzheng ! I was reviewing the function This is causing setting as required Note: I'm just trying to understand the project logic in order to collaborate in a future. |
@plusiv You are right! Unittest should be updated. |
This line is to respect to unittest and compare with old version logic, after tests code updated, if can be removed. |
Good! Seems razonable now!
Got it, I think it can be updated on this PR, or you think a new one is necessary @long2ice ?? I am willing to assist if you agree. |
Hey everyone, Did this PR got stuck? I think those are very importan changes. |
Thanks! Just need resolve conflicts. |
76c0f2a
to
530d6dd
Compare
conflicts fixed, but need you to rerun ci action manually. |
Thanks! |
Fix pydantic v2 pydantic_model_creator nullable field not optional(#1454)
How Has This Been Tested?
Checklist: