with pydantic v2, use pydantic.AwareDatetime instead of datetime

[gjcarneiro]

Openapi spec says that type string with format "date-time" must have a timezone. Therefore, we need pydantic.AwareDatetime here.

[codspeed-hq]

CodSpeed Performance Report

Merging #1735 will not alter performance

_{Comparing gjcarneiro:aware-datetime (8fcf101) with main (a36ce94)}

Summary

✅ 29 untouched benchmarks

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

❗ No coverage uploaded for pull request base (main@a36ce94). Click here to learn what that means.

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #1735   +/-   ##
=======================================
  Coverage        ?   99.05%           
=======================================
  Files           ?       37           
  Lines           ?     4112           
  Branches        ?      954           
=======================================
  Hits            ?     4073           
  Misses          ?       25           
  Partials        ?       14           

Flag	Coverage Δ
unittests	98.73% <0.00%> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[koxudaxi]

@gjcarneiro
https://datatracker.ietf.org/doc/html/rfc3339#section-5.6
Does this RFC mandate the inclusion of time zones?

[gjcarneiro]

Yes, it certainly does:

full-time = partial-time time-offset
date-time = full-date "T" full-time

(time-offset is the timezone)

I discovered this from validating my requests (with response coming from openapispec -> datamodel-codgen -> pydantic -> model_dump_json()) using the python package openapi-core. If you don't provide a timezone, it doesn't validate.

I guess there is a workaround without this PR: in pydantic, the datetime type accepts both timezone aware and naive datetime objects. As long as you always provide a datetime aware object, all is good. But it is more precise, and detects the issue earlier, to use pydantic.AwareDatetime.

[koxudaxi]

Thank you for explaining it.
I agree with you.

[koxudaxi]

The issue is fixed in the latest version.
Thank you very much!!

[pyssling]

Hi! This broke quite a few of our use-cases. Would it be possible to make this configurable, so one could toggle with a parameter between NaiveDatetime and AwareDatetime? @koxudaxi ?

[archetipo]

Any news on this topic? thanks

[gjcarneiro]

Note that date-time is defined by OpenAPI to always be timezone-aware. I still think it is right that datamodel-code-generator enforces that datetime objects must be timezone aware. You guys are asking for an option to make pydantic models not conformant to the spec?... 🤨

[archetipo]

I agree, but this is a pydantic tool, and pydantic provides for multiple types of data fields, forcing only one type into the code is not a correct approach, so in this case I am not arguing the correctness of the alignment to the RFC, but the hasty implementation..

[gjcarneiro]

I think that, in order to remain compatible with the spec, then the format specifier would have to be something other than format: date-time. It would have be a custom format, such as format: iso-date-time. ISO 8601 is more lenient and allows timezone to be optional. But OpenAPI uses RFC 3339 which is more strict.

[archetipo]

yes, but in this case pydantic provides differents data types, so they should be respected especially in case they are generated by a json schema , maybe generated by pydantic itself. In any case I think it is a good approach to add an option to the cli arguments, with which to drive to the given type of parser. For example:

case 1
datamodel-codegen --input file_name --input-file-type file_type --datetime-parser AwareDatetime

case 2
datamodel-codegen --input file_name --input-file-type file_type --datetime-parser NaiveDatetime

[gjcarneiro]

Yes, I know pydantic provides different data types. But, maybe I'm missing something, but the OpenAPI and JSONSchema inputs require the use of timezone-aware datetimes. Pydantic is more generic than OpenAPI/JSONSchema. And pydantic is the output format, it is never the input.

Unless I'm missing something and there is an input file api spec format that allows for timezone-naive datetime that datamodel-code-generator is supposed to support. I only ever use it with OpenAPI, and OpenAPI is pretty clear: timezone is not optional.

That said, if having such an option helps during a transition period to allow for migration of existing users, I don't think that's terrible. As long as it is clearly marked as a deprecated option and supported only for a few years until users migrate.

In the long term, it makes little sense to deviate from OpenAPI / JSONSchema standards IMHO. Disclaimer: I am not the maintainer, I just contributed this one PR. And apologies for breaking your code 😬

[pyssling]

Our fundamental problem is that we do not have timezones in the original data we are consuming, standards be damned. :-)

This breaks multiple projects and adds nothing of value for a data-consumer, only for a data-producer. I.e. Ideally I would like to be able to on the producer end enforce timezones, and on the consumer end accept whatever the server provides.

Refusing to decode data provided by a non-standards compliant server just isn't useful. :-/

[koxudaxi]

@pyssling

Hi! This broke quite a few of our use-cases. Would it be possible to make this configurable, so one could toggle with a parameter between NaiveDatetime and AwareDatetime? @koxudaxi ?

Yes, it is. There may need to be options for multiple use cases.

Thank you all for the discussion.
Without going into the details at once, what is the underlying problem with this?
Is it that you have not shown a specific design or how to use it?
One of the most often mentioned issues of this project is the lack of clarification of specifications and behavior.
This is because the use of the software is very often different from what was originally envisioned.

Would it be helpful to save everyone some time if there was a description of the behavior of the options in the documentation?
We need to think about how we should document the workings and use of the tool, but if it would be beneficial, it is worth considering.

[benverhees]

@koxudaxi setting aside the OpenApi specification, would you be open for a PR that would make this configurable?