feat: normalize use of chrono for time and datetime operations#2125
Open
feat: normalize use of chrono for time and datetime operations#2125
chrono for time and datetime operations#2125Conversation
chrono for time and datetime operationschrono for time and datetime operations
0d71dfc to
9b74b53
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Due to the age of the code and a number of other reasons, we have often “hand crafted” datetime and duration values as integers or strings instead of using well-known constants provided by existing core libraries. In addition, we also tend to use string or u64 representations of times instead of using Instance and Duration values, which can lead to errors and confusion.
This PR aims to make our use more consistent in how we deal with timestamps and time values, favoring
chronoyet maintaingstd::time::Durationfor those APIs like reqwest, deadpool, or actix and expect this in their signatures. We made these updates to Autopush also due to a history of bugs related to inconsistency.There is mixed use of the
timecrate,chronoand Rust's standardstd::timecrate. There are instances in which the signature is maintained, in the example oftokio::time, since it would make needless conversions that complicate the code for no benefit.The majority of use cases of
std::timeare related to capturing a duration since epoch, converting to milliseconds and then casting as an i64. ManyDurationvalues remain intact as they are used for type consistency. In instances where Durations are derived and passed into a function,Duration::from_secs()is used, opposed to setting aTimeDeltaand converting, which is cumbersome for these use cases.chronoDateTime<Utc>, timestamp formatting,TimeDelta(to be favored as preferential implementation)std::timeDuration(pool/server timeouts),Instant(metrics),SystemTime/UNIX_EPOCH(timestamp generation)timeDuration::weeks(52)in syncserver only, therefore removedtokio::timeAlso fixes an issue with Glean Probe scraper that had the wrong sha.
Issue(s)
Closes STOR-169.