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

Cannot Deserialize timestamp with milliseconds #647

Closed
ogarcia opened this issue Jan 26, 2024 · 3 comments · Fixed by #648
Closed

Cannot Deserialize timestamp with milliseconds #647

ogarcia opened this issue Jan 26, 2024 · 3 comments · Fixed by #648
Labels
A-third-party Area: implementations of traits from other crates C-feature-request Category: a new feature (not already implemented)

Comments

@ogarcia
Copy link
Contributor

ogarcia commented Jan 26, 2024

My code is simple:

use serde::{Deserialize, Serialize};
use time::OffsetDateTime;

// Adds milliseconds support to timestamps
time::serde::format_description!(ts_milliseconds, OffsetDateTime, "[unix_timestamp precision:millisecond]");

#[derive(Debug, Deserialize)]
pub struct Profile {
    pub id: String,
    pub name: String,
    #[serde(with = "ts_milliseconds")]
    pub createdAt: OffsetDateTime,
    #[serde(with = "ts_milliseconds")]
    pub updatedAt: OffsetDateTime,
}

When I go to use the structure to deserialize from a JSON I get the following error.

invalid type: integer `1617099399918`, expected a(n) `OffsetDateTime` in the format \"[unix_timestamp precision:millisecond]\"

The truth is that I have run out of ideas, I understand that it should work as it is, but it does not. In the cargo.toml I have the following:

time = { version = "~0.3", features = ["formatting", "macros", "parsing", "serde"] }
@jhpratt jhpratt added the C-question Category: seeking clarification on a topic label Jan 26, 2024
@jhpratt
Copy link
Member

jhpratt commented Jan 26, 2024

The value is expected to be a string, not an integer. Formatting and parsing is always done with strings.

@ogarcia
Copy link
Contributor Author

ogarcia commented Jan 27, 2024

The value is expected to be a string, not an integer. Formatting and parsing is always done with strings.

I understand then that it is impossible to deserialize a timestamp with milliseconds coming in an integer.

And you have not thought about including this possibility? It is not so rare nowadays to work with timestamps with milliseconds and the most normal thing is that they come as integers and not as strings.

In fact, I think it would be very interesting if it was even a module, something like time::serde::timestamp_millis. What do you think?

@jhpratt
Copy link
Member

jhpratt commented Jan 27, 2024

I have previously rejected that (on multiple occasions) because I assumed it could be replicated using the time::serde::format_description! macro. Given that it cannot and that time::serde::timestamp is serialized/deserialized as an integer, I would be fine with a PR adding it for milliseconds, microseconds, and nanoseconds.

@jhpratt jhpratt added C-feature-request Category: a new feature (not already implemented) A-third-party Area: implementations of traits from other crates and removed C-question Category: seeking clarification on a topic labels Jan 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-third-party Area: implementations of traits from other crates C-feature-request Category: a new feature (not already implemented)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants