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

implement Serialize/Deserialize for SystemTime with RFC3339 format #7203

Conversation

kevinmingtarja
Copy link
Contributor

Problem

We have two places that use a helper (ser_rfc3339_millis) to get serde to stringify SystemTimes into the desired format.

Summary of changes

Created a new module utils::serde_system_time and inside it a wrapper type SystemTime for std::time::SystemTime that serializes/deserializes to the RFC3339 format.

This new type is then used in the two places that were previously using the helper for serialization, thereby eliminating the need to decorate structs.

Closes #7151.

@kevinmingtarja kevinmingtarja requested a review from a team as a code owner March 21, 2024 20:01
@jcsp jcsp requested review from VladLazar and removed request for jcsp April 5, 2024 08:26
@jcsp
Copy link
Contributor

jcsp commented Apr 5, 2024

Thanks Kevin - @VladLazar could you review please?

Copy link
Contributor

@VladLazar VladLazar left a comment

Choose a reason for hiding this comment

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

Thanks for the PR. Looks good to me (just one minor nit).

libs/utils/src/serde_system_time.rs Outdated Show resolved Hide resolved
@VladLazar VladLazar added the approved-for-ci-run Changes are safe to trigger CI for the PR label Apr 5, 2024
@github-actions github-actions bot removed the approved-for-ci-run Changes are safe to trigger CI for the PR label Apr 5, 2024
@vipvap vipvap mentioned this pull request Apr 5, 2024
Copy link

github-actions bot commented Apr 5, 2024

2754 tests run: 2630 passed, 0 failed, 124 skipped (full report)


Flaky tests (2)

Postgres 16

  • test_multi_attach: debug

Postgres 15

  • test_vm_bit_clear_on_heap_lock: debug

Code coverage* (full report)

  • functions: 28.1% (6409 of 22846 functions)
  • lines: 46.9% (45114 of 96245 lines)

* collected from Rust tests only


The comment gets automatically updated with the latest test results
36bb8dd at 2024-04-08T10:30:42.879Z :recycle:

Copy link
Contributor

@VladLazar VladLazar left a comment

Choose a reason for hiding this comment

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

@kevinmingtarja looks like your PR raced with a change to the utilization endpoint. Could you please update your branch? I'll run CI again after that and merge it.

@kevinmingtarja
Copy link
Contributor Author

@kevinmingtarja looks like your PR raced with a change to the utilization endpoint. Could you please update your branch? I'll run CI again after that and merge it.

Got it, I just updated my branch and resolved the merge conflict.

@VladLazar VladLazar added the approved-for-ci-run Changes are safe to trigger CI for the PR label Apr 8, 2024
@github-actions github-actions bot removed the approved-for-ci-run Changes are safe to trigger CI for the PR label Apr 8, 2024
@VladLazar VladLazar merged commit a306d0a into neondatabase:main Apr 8, 2024
87 of 89 checks passed
@VladLazar
Copy link
Contributor

@kevinmingtarja thanks for the 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 this pull request may close these issues.

pageserver: implement Serialize for SystemTime with RFC3339 format
3 participants