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

impl Timestamp::{Display, FromStr} with RFC-3339 format #615

Merged
merged 7 commits into from
Apr 14, 2022

Conversation

danburkert
Copy link
Collaborator

This PR implements to and from string methods for
prost_types::Timestamp following the RFC-3339 timestamp format, as
also specified by the Protobuf JSON mapping spec. The Display
impl always emits the Zulu (Z) timezone, whereas the parser is slightly
lenient in accepting fixed timezone offsets, as well as both the T and
space seperated date and time portions. This leniency in parsing makes
it much much more robust to parsing data of less-than-pristine
formatting, even though the protobuf spec allows no leniency.

Also, since it was easy with the new date time functionality, we've gone
ahead and added a trio of convenience construct functions to Timestamp
which makes it straightforward to create Timestamps out of calendar
date time information.

This functionality will be used in follow-up PRs to add Serde
[de]serialization support to the types in prost_types, which is itself
a stepping stone towards full JSON support.

This PR was extracted from #558, and is based on code which myself and
@konradjniemiec have written, and which has been running in production
at @sisudata for a few years. The to-string portion of this code has
also been incorporated into tracing, so it should be pretty well
battle hardened.

@danburkert danburkert requested a review from LucioFranco April 1, 2022 21:23
@danburkert
Copy link
Collaborator Author

CC @neoeinstein

prost-types/src/lib.rs Outdated Show resolved Hide resolved
prost-types/src/datetime.rs Outdated Show resolved Hide resolved
Copy link
Member

@LucioFranco LucioFranco left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM mostly one question and also gonna really trust proptest on this one

prost-types/src/datetime.rs Show resolved Hide resolved
prost-types/src/datetime.rs Outdated Show resolved Hide resolved
prost-types/src/lib.rs Outdated Show resolved Hide resolved
@danburkert danburkert requested a review from LucioFranco April 4, 2022 23:48
@danburkert danburkert force-pushed the prost-types-timestamp branch from 91081d3 to 9ea85ff Compare April 5, 2022 00:57
@LucioFranco
Copy link
Member

@danburkert is this ready for final review?

@danburkert
Copy link
Collaborator Author

Yes, this is ready for final review. Thanks @LucioFranco !

danburkert added a commit to sisudata/prost that referenced this pull request Apr 7, 2022
This PR implements to and from string methods for
`prost_types::Duration` following the duration format defined by the
[Protobuf JSON mapping spec][1].

This is a followup to tokio-rs#615 and re-uses a lot of the same parser helpers.

[1]: https://developers.google.com/protocol-buffers/docs/proto3#json
This PR implements to and from string methods for
`prost_types::Timestamp` following the RFC-3339 timestamp format, as
also specified by the [Protobuf JSON mapping spec][1]. The `Display`
impl always emits the Zulu (Z) timezone, whereas the parser is slightly
lenient in accepting fixed timezone offsets, as well as both the `T` and
space seperated date and time portions. This leniency in parsing makes
it much much more robust to parsing data of less-than-pristine
formatting, even though the protobuf spec allows no leniency.

Also, since it was easy with the new date time functionality, we've gone
ahead and added a trio of convenience construct functions to `Timestamp`
which makes it straightforward to create `Timestamp`s out of calendar
date time information.

This functionality will be used in follow-up PRs to add Serde
[de]serialization support to the types in `prost_types`, which is itself
a stepping stone towards full JSON support.

This PR was extracted from tokio-rs#558, and is based on code which myself and
@konradjniemiec have written, and which has been running in production
at @sisudata for a few years. The to-string portion of this code has
also been incorporated into [`tracing`][2], so it should be pretty well
battle hardened.

[1]: https://developers.google.com/protocol-buffers/docs/proto3#json
[2]: https://github.com/tokio-rs/tracing/blob/master/tracing-subscriber/src/fmt/time/datetime.rs
@danburkert danburkert force-pushed the prost-types-timestamp branch from a109f14 to e829806 Compare April 14, 2022 18:14
@danburkert danburkert merged commit 1674ef9 into tokio-rs:master Apr 14, 2022
danburkert added a commit that referenced this pull request Apr 14, 2022
This PR implements to and from string methods for
`prost_types::Duration` following the duration format defined by the
[Protobuf JSON mapping spec][1].

This is a followup to #615 and re-uses a lot of the same parser helpers.
Additionally I did the same error type unification with a new
`DurationError` type. This is a breaking change for `prost-types` since
public APIs are changed slightly.

[1]: https://developers.google.com/protocol-buffers/docs/proto3#json
danburkert added a commit that referenced this pull request Apr 20, 2022
This PR implements to and from string methods for
`prost_types::Duration` following the duration format defined by the
[Protobuf JSON mapping spec][1].

This is a followup to #615 and re-uses a lot of the same parser helpers.
Additionally I did the same error type unification with a new
`DurationError` type. This is a breaking change for `prost-types` since
public APIs are changed slightly.

[1]: https://developers.google.com/protocol-buffers/docs/proto3#json
benesch added a commit to MaterializeInc/prost that referenced this pull request May 21, 2022
mdrach pushed a commit to sisudata/prost that referenced this pull request Aug 3, 2022
This PR implements to and from string methods for
`prost_types::Duration` following the duration format defined by the
[Protobuf JSON mapping spec][1].

This is a followup to tokio-rs#615 and re-uses a lot of the same parser helpers.

[1]: https://developers.google.com/protocol-buffers/docs/proto3#json
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 this pull request may close these issues.

2 participants