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

fix: Rust pipeline #4133

Merged
merged 6 commits into from
Aug 14, 2023
Merged

fix: Rust pipeline #4133

merged 6 commits into from
Aug 14, 2023

Conversation

jkomyno
Copy link
Contributor

@jkomyno jkomyno commented Aug 11, 2023

This PR:

We note that, previously:

@codspeed-hq
Copy link

codspeed-hq bot commented Aug 11, 2023

CodSpeed Performance Report

Merging #4133 will degrade performances by 15.29%

Comparing fix/silence-deprecations (6ca30d5) with main (9fe3fe4)

Summary

❌ 3 regressions
✅ 8 untouched benchmarks

⚠️ Please fix the performance issues or acknowledge them on CodSpeed.

Benchmarks breakdown

Benchmark main fix/silence-deprecations Change
validate (small) 8.1 ms 9.5 ms -15.29%
validate (medium) 58.3 ms 66.4 ms -12.24%
validate (large) 270.6 ms 298.4 ms -9.33%

@jkomyno jkomyno changed the title fix: silence deprecations fix: Rust pipeline Aug 11, 2023
@@ -364,7 +364,7 @@ fn read_scalar_value(bson: Bson, meta: &ScalarOutputMeta) -> crate::Result<Prism
// DateTime
(TypeIdentifier::DateTime, Bson::DateTime(dt)) => PrismaValue::DateTime(dt.to_chrono().into()),
(TypeIdentifier::DateTime, Bson::Timestamp(ts)) => {
PrismaValue::DateTime(Utc.timestamp(ts.time as i64, 0).into())
PrismaValue::DateTime(Utc.timestamp_opt(ts.time as i64, 0).unwrap().into())
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm pasting here the implementation of Utc::timestamp for the reviewer's convenience:

    /// Makes a new `DateTime` from the number of non-leap seconds
    /// since January 1, 1970 0:00:00 UTC (aka "UNIX timestamp")
    /// and the number of nanoseconds since the last whole non-leap second.
    ///
    /// Panics on the out-of-range number of seconds and/or invalid nanosecond,
    /// for a non-panicking version see [`timestamp_opt`](#method.timestamp_opt).
    #[deprecated(since = "0.4.23", note = "use `timestamp_opt()` instead")]
    fn timestamp(&self, secs: i64, nsecs: u32) -> DateTime<Self> {
        self.timestamp_opt(secs, nsecs).unwrap()
    }

@jkomyno jkomyno added this to the 5.2.0 milestone Aug 11, 2023
@jkomyno jkomyno marked this pull request as ready for review August 11, 2023 14:43
@jkomyno jkomyno requested a review from a team as a code owner August 11, 2023 14:43
@jkomyno jkomyno requested a review from SevInf August 11, 2023 14:43
@tomhoule
Copy link
Contributor

Copy-pasting my comment from slack here:

I'm a bit confused by the codspeed report though, the only thing that could have caused it in this PR is the dependency updates. Does that sound likely, or could it be because of a measurement problem? I'd like us to figure that out before merging.

@jkomyno jkomyno merged commit 43696fa into main Aug 14, 2023
47 of 48 checks passed
@jkomyno jkomyno deleted the fix/silence-deprecations branch August 14, 2023 08:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants