scylladb/scylla-rust-driver

Clippy fails because of chrono deprecation

Lorak-mmk opened this issue · 1 comments

New version of chrono was released a few hours ago.
It deprecated chrono::Duration::days, pointing to chrono::Duration::try_days instead.
This deprecation causes our CI to fail:
https://github.com/scylladb/scylla-rust-driver/actions/runs/8178066760/job/22361165132?pr=948#step:5:398

error: use of deprecated associated function `chrono::TimeDelta::days`: Use `TimeDelta::try_days` instead
   --> scylla-cql/src/frame/value.rs:492:59
    |
492 |         let duration_since_unix_epoch = chrono::Duration::days(days_since_unix_epoch);
    |                                                           ^^^^
    |
    = note: `-D deprecated` implied by `-D warnings`
    = help: to override `-D warnings` add `#[allow(deprecated)]`

try_days was introduced in chrono 0.4.33 (or 0.4.32 - but docs.rs for this version don't build, so I didn't check).
In order to migrate, we need to bump our chrono dependency to 0.4.33 from the current 0.4.20 - so we need to require a very recent release. I'm a bit disappointed that they deprecate stuff so quickly after introducing replacement...

The alternative is to ignore this clippy error in this place, but I don't like this either.

This is the place where we use this method:

#[cfg(feature = "chrono")]
impl TryInto<NaiveDate> for CqlDate {
    type Error = ValueOverflow;

    fn try_into(self) -> Result<NaiveDate, Self::Error> {
        let days_since_unix_epoch = self.0 as i64 - (1 << 31);

        // date_days is u32 then converted to i64 then we subtract 2^31;
        // Max value is 2^31, min value is -2^31. Both values can safely fit in chrono::Duration, this call won't panic
        let duration_since_unix_epoch = chrono::Duration::days(days_since_unix_epoch);

        NaiveDate::from_yo_opt(1970, 1)
            .unwrap()
            .checked_add_signed(duration_since_unix_epoch)
            .ok_or(ValueOverflow)
    }
}

Given that this call can never fail, I'm wondering if it's better to unwrap() or return ValueOverflow.
I think I prefer unwrapping - panic will mean than there is an error in our code (or in the comment) which should be fixed.
Returnig ValueOverflow will not allow to distinguish between checked_add_signed fail and try_days fail.

Oh, it's not the only deprecation. When running CI locally I see more:

error: use of deprecated associated function `chrono::TimeDelta::days`: Use `TimeDelta::try_days` instead
   --> scylla-cql/src/frame/value.rs:492:59
    |
492 |         let duration_since_unix_epoch = chrono::Duration::days(days_since_unix_epoch);
    |                                                           ^^^^
    |
    = note: `-D deprecated` implied by `-D warnings`
    = help: to override `-D warnings` add `#[allow(deprecated)]`

error: use of deprecated associated function `chrono::NaiveDateTime::from_timestamp_opt`: use `DateTime::from_timestamp` instead
    --> scylla-cql/src/frame/response/result.rs:1586:41
     |
1586 |         let unix_epoch = NaiveDateTime::from_timestamp_opt(0, 0).unwrap().and_utc();
     |                                         ^^^^^^^^^^^^^^^^^^
     |
     = note: `-D deprecated` implied by `-D warnings`
     = help: to override `-D warnings` add `#[allow(deprecated)]`

error: use of deprecated associated function `chrono::TimeDelta::days`: Use `TimeDelta::try_days` instead
   --> scylla-cql/src/frame/value.rs:492:59
    |
492 |         let duration_since_unix_epoch = chrono::Duration::days(days_since_unix_epoch);
    |                                                           ^^^^

error: use of deprecated method `chrono::NaiveDateTime::timestamp_millis`: use `.and_utc().timestamp_millis()` instead
   --> scylla-cql/src/frame/value_tests.rs:498:32
    |
498 |             NaiveDateTime::MAX.timestamp_millis().to_be_bytes(),
    |                                ^^^^^^^^^^^^^^^^

error: use of deprecated method `chrono::NaiveDateTime::timestamp_millis`: use `.and_utc().timestamp_millis()` instead
   --> scylla-cql/src/frame/value_tests.rs:503:32
    |
503 |             NaiveDateTime::MIN.timestamp_millis().to_be_bytes(),
    |                                ^^^^^^^^^^^^^^^^

error: use of deprecated associated function `chrono::NaiveDateTime::from_timestamp_opt`: use `DateTime::from_timestamp` instead
   --> scylla-cql/src/frame/value_tests.rs:507:28
    |
507 |             NaiveDateTime::from_timestamp_opt(0, 0).unwrap(),
    |                            ^^^^^^^^^^^^^^^^^^

error: use of deprecated associated function `chrono::NaiveDateTime::from_timestamp_opt`: use `DateTime::from_timestamp` instead
   --> scylla-cql/src/frame/value_tests.rs:512:28
    |
512 |             NaiveDateTime::from_timestamp_opt(1, 0).unwrap(),
    |                            ^^^^^^^^^^^^^^^^^^

error: use of deprecated associated function `chrono::NaiveDateTime::from_timestamp_opt`: use `DateTime::from_timestamp` instead
   --> scylla-cql/src/frame/value_tests.rs:517:28
    |
517 |             NaiveDateTime::from_timestamp_opt(0, 1).unwrap(),
    |                            ^^^^^^^^^^^^^^^^^^

error: use of deprecated associated function `chrono::NaiveDateTime::from_timestamp_opt`: use `DateTime::from_timestamp` instead
   --> scylla-cql/src/frame/value_tests.rs:522:28
    |
522 |             NaiveDateTime::from_timestamp_opt(0, 1_000_000).unwrap(),
    |                            ^^^^^^^^^^^^^^^^^^

error: use of deprecated associated function `chrono::NaiveDateTime::from_timestamp_opt`: use `DateTime::from_timestamp` instead
   --> scylla-cql/src/frame/value_tests.rs:527:28
    |
527 |             NaiveDateTime::from_timestamp_opt(-2 * 24 * 60 * 60, 0).unwrap(),
    |                            ^^^^^^^^^^^^^^^^^^

chrono has quite a lot of deprecations and other changes lately