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

Instant instead of ZonedDateTime #59

Closed
agentgonzo opened this issue Sep 6, 2022 · 1 comment
Closed

Instant instead of ZonedDateTime #59

agentgonzo opened this issue Sep 6, 2022 · 1 comment

Comments

@agentgonzo
Copy link
Member

Instant is a more ideomatic way of representing times in java (akin to unix time). It's a timezone-less entity (ie, UTC).

There are a bunch of helper functions to do any conversions you may want, as well as to/from ISO-8601 zulu-time strings.

ZonedDateTime contains timezone information, which (whilst useful) can introduce confusion when dealing with distributed systems. Ideally, all backend/server code (ie, Java) would use Instant (ie, fancy-formatted unix time) for all datetime representations, then leave it up to the frontend to worry about formatting with specific timezones.

I think it's worth changing https://github.com/open-feature/java-sdk/blob/main/src/main/java/dev/openfeature/javasdk/Value.java#L49-L51 to be using Instant instead of ZonedDateTime

@toddbaert
Copy link
Member

I'm in agreement with this suggestion, though we also had a brief discussion on whether or not we wanted to include timestamps as "first class" data in the context or structure return values at all.

I think I'd like to keep timestamps as a first class type in the context, but I'm interested if others think it's worth the burden.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants