Skip to content

Optional[datetime] field with value null/None gets isoparse'd #456

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

Closed
cfal opened this issue Jul 21, 2021 · 5 comments
Closed

Optional[datetime] field with value null/None gets isoparse'd #456

cfal opened this issue Jul 21, 2021 · 5 comments
Labels
🐞bug Something isn't working

Comments

@cfal
Copy link

cfal commented Jul 21, 2021

I have a field in my generated model like this:

timestamp: Union[Unset, datetime.datetime] = UNSET

My API returns {"timestamp": null}, which then gets deserialized as None, and ends up in the else clause of this generated code:

        if isinstance(_timestamp, Unset):
            timestamp = UNSET
        else:
            timestamp = isoparse(_timestamp)

.. when then ends up raising in dateutil's isoparser.py.

I'm using 0.10.1 and saw #381 and #420 - wondering if datetime.datetime wasn't fixed with that issue or if I'm doing something wrong?

@cfal cfal added the 🐞bug Something isn't working label Jul 21, 2021
@dbanty
Copy link
Collaborator

dbanty commented Jul 21, 2021

If the type of the generated model is Union[Unset, datetime.datetime] then I’m guessing the schema doesn’t have nullable set. In which case, null is an invalid value for the server to be returning according to the schema.

See #385 for more info on the required vs nullable difference in spec.

If your schema does have nullable: true in it then it sounds like a generation bug, as the type should be Union[Unset, datetime.datetime, None] (if not required) or Optional[datetime.datetime] (if required).

@cfal
Copy link
Author

cfal commented Jul 21, 2021

thanks for the quick response!

yeah, I was indeed missing nullable fields :) adding it changed the parameter from Union[Unset, datetime.datetime] to Optional[datetime.datetime]. Unfortunately that also caused an additional problem - ironically enough it seems the parameter is no longer optional when instantiating the model class.

for example, in the generated from_dict method, the line changed from:

timestamp = d.pop("timestamp", UNSET)

to

timestamp = d.pop("timestamp")

which now raises with a KeyError if timestamp is not found. and similarly, a timestamp argument now needs to be specified if instantiating via the constructor.

@dbanty
Copy link
Collaborator

dbanty commented Jul 21, 2021

Sounds like now it thinks the parameter is in the required list. Did you happen to add it there when you added nullable?

@dbanty
Copy link
Collaborator

dbanty commented Jul 21, 2021

Just to be a little more clear, I believe you have a schema that looks something like this (depending on where it's defined). The property being nullable and also required means that it has to be set, but could be set to null. If you remove it from required then it could be null, but could also be omitted (leading to UNSET).

"AModel": {
    "required": [
         "timestamp"
    ],
    "type": "object",
    "properties": {
        "timestamp": {
            "type": "string",
            "format": "date-time",
            "nullable": true
        }
    }
}

@cfal
Copy link
Author

cfal commented Jul 22, 2021

thanks @dbanty, that was the problem. I'm using pydantic/fastapi, and adding = pydantic.Field(..., nullable=True) to the field somehow ended up making it required in the openapi output. appreciate the help!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🐞bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants