Skip to content
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

format: support formatting time.Time as RFC3339Nano #1063

Closed
anuraaga opened this issue Oct 5, 2023 · 10 comments · Fixed by #1334
Closed

format: support formatting time.Time as RFC3339Nano #1063

anuraaga opened this issue Oct 5, 2023 · 10 comments · Fixed by #1334
Labels
enhancement New feature or request openapi-features OpenAPI features support issues

Comments

@anuraaga
Copy link

anuraaga commented Oct 5, 2023

Description

Currently, date-time format uses RFC3339. It would be nice if it were possible to format is RFC3339Nano, perhaps by adding a custom date-time-nano format, or adding a generator option. While there is naturally a limit to how many bespoke formats this generator can support, I believe this one is particularly helpful because the default JSON marshaling of Golang for time.Time is actually RFC3339Nano, which means that there is some friction in migrating existing codebases to ogen which may be using default JSON marshaling, even via some framework like Gin, and having no way without CustomFormat to reproduce the same behavior with ogen.

@anuraaga anuraaga added enhancement New feature or request openapi-features OpenAPI features support issues labels Oct 5, 2023
@chaporgin
Copy link

chaporgin commented Nov 9, 2023

Could anyone please have a look at that?

@anuraaga
Copy link
Author

Updating ogen after a while and realized that CustomFormat support was removed in #1090. It's too bad since it seemed like a useful feature in certain situations, but specifically for this now it seems there may be no way to reproduce the standard library marshaling of time.Time with ogen. The format idea seems nice too, would it be a reasonable feature to add?

@abemedia
Copy link
Contributor

This is also becoming a higher priority for me.
Would you accept a PR that adds an option to the ogen.yaml to set a default format? @ernado @tdakkota @shadowspore

@tdakkota
Copy link
Member

@abemedia

Sure, why not.

I was thinking of extension like x-ogen-time-format as it might be a more general solution, though.

@abemedia
Copy link
Contributor

@tdakkota that would work too. We actually need nano precision on all our timestamps though, so how would you feel about having both?

If you'd rather have only the extension that's fine too as we could just create a custom type with the extension and reference that everywhere we need a timestamp but the former would be a nice touch.

@tdakkota
Copy link
Member

I am fine with config option. We may add an extension later, if there is some interest to it.

@abemedia
Copy link
Contributor

Would you be in favour of an option that just switches it to nano (i.e. a boolean option), or would you prefer that people can input any format string?
Technically other formats than RFC3339 and RFC3339Nano aren't valid as the OpenAPI spec clearly defines it as RFC3339, however some people might not care about that.

@abemedia
Copy link
Contributor

Friendly bump @tdakkota 🙂

@tdakkota
Copy link
Member

A string option seems to be a better solution, since it is useful in case when third-party APIs/client use an odd date-time represention.

@abemedia
Copy link
Contributor

@tdakkota I opted for your idea of an extension after all as it seemed simpler to implement: #1334
This does actually address my use-case as I can simply create a shared schema and reference that everywhere I want a date-time.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request openapi-features OpenAPI features support issues
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants