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

Add timestamp_micros to Utc #1284

Closed
emikitas opened this issue Sep 12, 2023 · 4 comments · Fixed by #1285
Closed

Add timestamp_micros to Utc #1284

emikitas opened this issue Sep 12, 2023 · 4 comments · Fixed by #1285

Comments

@emikitas
Copy link
Contributor

emikitas commented Sep 12, 2023

I wanted to know if there is a reason for having the functions timestamp_millis and timestamp_nanos in the trait TimeZone but not timestamp_micros.
If there is no reason not to have it, will it be possible to implement it?

@djc
Copy link
Member

djc commented Sep 12, 2023

What's your use case for it?

@emikitas
Copy link
Contributor Author

I wanted to add support for processing microseconds in VRL, specifically in the following function:
https://github.com/vectordotdev/vrl/blob/000109385569314c6715300bfd84cba0971646c3/src/stdlib/from_unix_timestamp.rs#L5
I figured I might not be the only person interested in transforming microsecond timestamps into DateTime, so I figured I could add it to chrono.
I'd like to implement it myself, but I first wanted to check if there is a reason not to.

@pitdicker
Copy link
Collaborator

timestamp_millis was added in #268.
Personally I am not a fan of the large number of timestamp_* methods we are getting. But one more timestamp_micros (that returns a LocalResult like timestamp_millis_opt) would make the set complete 🤷.

Maybe, if you add it: can you also change TimeZone::timestamp_millis_opt to use NaiveDateTime::from_timestamp_millis under the hood? So that we don't have the same functionality with two different implementations.

@djc
Copy link
Member

djc commented Sep 12, 2023

Okay, please submit a PR.

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

Successfully merging a pull request may close this issue.

3 participants