From a6870c161d74fbc26948d8df00ebd49f605edbe6 Mon Sep 17 00:00:00 2001 From: Lei Xu Date: Sat, 9 Mar 2024 11:46:51 -0800 Subject: [PATCH] chore: upgrade chrono and fix deprecation warnings (#2048) Bump chrono to 0.4.25 and fix warnings. --- Cargo.toml | 5 +- rust/lance-core/src/utils/testing.rs | 4 +- rust/lance-datagen/src/generator.rs | 4 +- rust/lance-table/src/format/manifest.rs | 4 +- rust/lance/src/dataset/cleanup.rs | 64 ++++++++++++++++--------- rust/lance/src/utils/temporal.rs | 7 +-- 6 files changed, 56 insertions(+), 32 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 6ac073186f..b3208d73c1 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -78,7 +78,10 @@ half = { "version" = "=2.3.1", default-features = false, features = [ bytes = "1.4" byteorder = "1.5" clap = { version = "4", features = ["derive"] } -chrono = "0.4.23" +chrono = { version = "0.4.25", default-features = false, features = [ + "std", + "now" +] } criterion = { version = "0.5", features = ["async", "async_tokio"] } datafusion = { version = "35.0.0", default-features = false, features = [ "regex_expressions", diff --git a/rust/lance-core/src/utils/testing.rs b/rust/lance-core/src/utils/testing.rs index 16a081efc5..e103a18349 100644 --- a/rust/lance-core/src/utils/testing.rs +++ b/rust/lance-core/src/utils/testing.rs @@ -17,7 +17,7 @@ use crate::Result; use async_trait::async_trait; use bytes::Bytes; -use chrono::Duration; +use chrono::{Duration, TimeDelta}; use futures::stream::BoxStream; use futures::{StreamExt, TryStreamExt}; use object_store::path::Path; @@ -252,6 +252,6 @@ impl<'a> MockClock<'a> { impl<'a> Drop for MockClock<'a> { fn drop(&mut self) { // Reset the clock to the epoch - mock_instant::MockClock::set_system_time(Duration::days(0).to_std().unwrap()); + mock_instant::MockClock::set_system_time(TimeDelta::try_days(0).unwrap().to_std().unwrap()); } } diff --git a/rust/lance-datagen/src/generator.rs b/rust/lance-datagen/src/generator.rs index 89f6979c75..0580b5cedb 100644 --- a/rust/lance-datagen/src/generator.rs +++ b/rust/lance-datagen/src/generator.rs @@ -1054,7 +1054,7 @@ pub mod array { /// is a more common use pattern pub fn rand_date32() -> Box { let now = chrono::Utc::now(); - let one_year_ago = now - chrono::Duration::days(365); + let one_year_ago = now - chrono::TimeDelta::try_days(365).expect("TimeDelta try days"); rand_date32_in_range(one_year_ago, now) } @@ -1087,7 +1087,7 @@ pub mod array { /// is a more common use pattern pub fn rand_date64() -> Box { let now = chrono::Utc::now(); - let one_year_ago = now - chrono::Duration::days(365); + let one_year_ago = now - chrono::TimeDelta::try_days(365).expect("TimeDelta try_days"); rand_date64_in_range(one_year_ago, now) } diff --git a/rust/lance-table/src/format/manifest.rs b/rust/lance-table/src/format/manifest.rs index 50bc602db7..2c0c7eca8b 100644 --- a/rust/lance-table/src/format/manifest.rs +++ b/rust/lance-table/src/format/manifest.rs @@ -143,7 +143,9 @@ impl Manifest { let nanos = self.timestamp_nanos % 1_000_000_000; let seconds = ((self.timestamp_nanos - nanos) / 1_000_000_000) as i64; Utc.from_utc_datetime( - &NaiveDateTime::from_timestamp_opt(seconds, nanos as u32).unwrap_or(NaiveDateTime::MIN), + &DateTime::from_timestamp(seconds, nanos as u32) + .unwrap_or_default() + .naive_utc(), ) } diff --git a/rust/lance/src/dataset/cleanup.rs b/rust/lance/src/dataset/cleanup.rs index 128daaded6..fd066ef200 100644 --- a/rust/lance/src/dataset/cleanup.rs +++ b/rust/lance/src/dataset/cleanup.rs @@ -44,7 +44,7 @@ //! (which should only be done if the caller can guarantee there are no updates //! happening at the same time) -use chrono::{DateTime, Duration, Utc}; +use chrono::{DateTime, TimeDelta, Utc}; use futures::{stream, StreamExt, TryStreamExt}; use lance_core::{Error, Result}; use lance_io::object_store::ObjectStore; @@ -217,7 +217,8 @@ impl<'a> CleanupTask<'a> { inspection: CleanupInspection, ) -> Result { let removal_stats = Mutex::new(RemovalStats::default()); - let verification_threshold = utc_now() - Duration::days(UNVERIFIED_THRESHOLD_DAYS); + let verification_threshold = utc_now() + - TimeDelta::try_days(UNVERIFIED_THRESHOLD_DAYS).expect("TimeDelta::try_days"); let unreferenced_paths = self .dataset .object_store @@ -455,7 +456,6 @@ mod tests { use arrow::datatypes::{DataType, Field, Schema as ArrowSchema}; use arrow_array::{RecordBatchIterator, RecordBatchReader}; - use chrono::Duration; use lance_core::{ utils::testing::{MockClock, ProxyObjectStore, ProxyObjectStorePolicy}, Error, Result, @@ -745,12 +745,14 @@ mod tests { fixture.create_some_data().await.unwrap(); fixture.overwrite_some_data().await.unwrap(); - fixture.clock.set_system_time(Duration::days(10)); + fixture + .clock + .set_system_time(TimeDelta::try_days(10).unwrap()); let before_count = fixture.count_files().await.unwrap(); let removed = fixture - .run_cleanup(utc_now() - Duration::days(8)) + .run_cleanup(utc_now() - TimeDelta::try_days(8).unwrap()) .await .unwrap(); @@ -784,7 +786,9 @@ mod tests { // remain if they are still referenced by newer manifests let fixture = MockDatasetFixture::try_new().unwrap(); fixture.create_some_data().await.unwrap(); - fixture.clock.set_system_time(Duration::days(10)); + fixture + .clock + .set_system_time(TimeDelta::try_days(10).unwrap()); fixture.append_some_data().await.unwrap(); fixture.append_some_data().await.unwrap(); @@ -794,7 +798,7 @@ mod tests { assert_eq!(before_count.num_data_files, 3); assert_eq!(before_count.num_manifest_files, 4); - let before = utc_now() - Duration::days(7); + let before = utc_now() - TimeDelta::try_days(7).unwrap(); let removed = fixture.run_cleanup(before).await.unwrap(); let after_count = fixture.count_files().await.unwrap(); @@ -817,7 +821,9 @@ mod tests { async fn cleanup_recent_verified_files() { let fixture = MockDatasetFixture::try_new().unwrap(); fixture.create_some_data().await.unwrap(); - fixture.clock.set_system_time(Duration::seconds(1)); + fixture + .clock + .set_system_time(TimeDelta::try_seconds(1).unwrap()); fixture.overwrite_some_data().await.unwrap(); let before_count = fixture.count_files().await.unwrap(); @@ -854,9 +860,9 @@ mod tests { assert!(fixture.append_some_data().await.is_err()); let age = if old_files { - Duration::days(UNVERIFIED_THRESHOLD_DAYS + 1) + TimeDelta::try_days(UNVERIFIED_THRESHOLD_DAYS + 1).unwrap() } else { - Duration::days(UNVERIFIED_THRESHOLD_DAYS - 1) + TimeDelta::try_days(UNVERIFIED_THRESHOLD_DAYS - 1).unwrap() }; fixture.clock.set_system_time(age); @@ -896,7 +902,9 @@ mod tests { let fixture = MockDatasetFixture::try_new().unwrap(); fixture.create_some_data().await.unwrap(); fixture.create_some_index().await.unwrap(); - fixture.clock.set_system_time(Duration::days(10)); + fixture + .clock + .set_system_time(TimeDelta::try_days(10).unwrap()); fixture.overwrite_some_data().await.unwrap(); let before_count = fixture.count_files().await.unwrap(); @@ -906,7 +914,7 @@ mod tests { // Creating an index creates a new manifest so there are 4 total assert_eq!(before_count.num_manifest_files, 4); - let before = utc_now() - Duration::days(8); + let before = utc_now() - TimeDelta::try_days(8).unwrap(); let removed = fixture.run_cleanup(before).await.unwrap(); let after_count = fixture.count_files().await.unwrap(); @@ -934,7 +942,9 @@ mod tests { // This will keep some data from the appended file and should // completely remove the first file fixture.delete_data("filter_me < 20").await.unwrap(); - fixture.clock.set_system_time(Duration::days(10)); + fixture + .clock + .set_system_time(TimeDelta::try_days(10).unwrap()); fixture.overwrite_data(data_gen.batch(16)).await.unwrap(); // This will delete half of the last fragment fixture.delete_data("filter_me >= 40").await.unwrap(); @@ -944,7 +954,7 @@ mod tests { assert_eq!(before_count.num_delete_files, 2); assert_eq!(before_count.num_manifest_files, 6); - let before = utc_now() - Duration::days(8); + let before = utc_now() - TimeDelta::try_days(8).unwrap(); let removed = fixture.run_cleanup(before).await.unwrap(); let after_count = fixture.count_files().await.unwrap(); @@ -970,12 +980,14 @@ mod tests { // by any fragment. We need to make sure the cleanup routine // doesn't over-zealously delete these let fixture = MockDatasetFixture::try_new().unwrap(); - fixture.clock.set_system_time(Duration::days(10)); + fixture + .clock + .set_system_time(TimeDelta::try_days(10).unwrap()); fixture.create_some_data().await.unwrap(); fixture.create_some_index().await.unwrap(); let before_count = fixture.count_files().await.unwrap(); - let before = utc_now() - Duration::days(8); + let before = utc_now() - TimeDelta::try_days(8).unwrap(); let removed = fixture.run_cleanup(before).await.unwrap(); assert_eq!(removed.old_versions, 0); assert_eq!(removed.bytes_removed, 0); @@ -994,7 +1006,9 @@ mod tests { fixture.create_some_data().await.unwrap(); fixture.block_commits(); assert!(fixture.append_some_data().await.is_err()); - fixture.clock.set_system_time(Duration::days(10)); + fixture + .clock + .set_system_time(TimeDelta::try_days(10).unwrap()); let before_count = fixture.count_files().await.unwrap(); // This append will fail since the commit is blocked but it should have @@ -1006,7 +1020,7 @@ mod tests { // All of our manifests are newer than the threshold but temp files // should still be deleted. let removed = fixture - .run_cleanup(utc_now() - Duration::days(7)) + .run_cleanup(utc_now() - TimeDelta::try_days(7).unwrap()) .await .unwrap(); @@ -1031,7 +1045,9 @@ mod tests { // but the cleanup routine has no way of detecting this. They should look // just like an in-progress write. let mut fixture = MockDatasetFixture::try_new().unwrap(); - fixture.clock.set_system_time(Duration::days(10)); + fixture + .clock + .set_system_time(TimeDelta::try_days(10).unwrap()); fixture.create_some_data().await.unwrap(); fixture.block_commits(); assert!(fixture.append_some_data().await.is_err()); @@ -1039,7 +1055,7 @@ mod tests { let before_count = fixture.count_files().await.unwrap(); let removed = fixture - .run_cleanup(utc_now() - Duration::days(7)) + .run_cleanup(utc_now() - TimeDelta::try_days(7).unwrap()) .await .unwrap(); @@ -1056,7 +1072,9 @@ mod tests { // prevent us from running cleanup again later. let mut fixture = MockDatasetFixture::try_new().unwrap(); fixture.create_some_data().await.unwrap(); - fixture.clock.set_system_time(Duration::days(10)); + fixture + .clock + .set_system_time(TimeDelta::try_days(10).unwrap()); fixture.overwrite_some_data().await.unwrap(); // The delete operation should delete the first version and its @@ -1069,7 +1087,7 @@ mod tests { assert_eq!(before_count.num_manifest_files, 3); assert!(fixture - .run_cleanup(utc_now() - Duration::days(7)) + .run_cleanup(utc_now() - TimeDelta::try_days(7).unwrap()) .await .is_err()); @@ -1085,7 +1103,7 @@ mod tests { fixture.unblock_delete_manifest(); let removed = fixture - .run_cleanup(utc_now() - Duration::days(7)) + .run_cleanup(utc_now() - TimeDelta::try_days(7).unwrap()) .await .unwrap(); diff --git a/rust/lance/src/utils/temporal.rs b/rust/lance/src/utils/temporal.rs index 103015d453..81b2cd8ebc 100644 --- a/rust/lance/src/utils/temporal.rs +++ b/rust/lance/src/utils/temporal.rs @@ -19,7 +19,7 @@ //! unit tests. Anywhere in production code where we need to get the current time //! we should use the below methods and types instead of the builtin methods and types -use chrono::{DateTime, NaiveDateTime, TimeZone, Utc}; +use chrono::{DateTime, TimeZone, Utc}; #[cfg(test)] use mock_instant::{SystemTime as NativeSystemTime, UNIX_EPOCH}; @@ -34,8 +34,9 @@ pub fn utc_now() -> DateTime { let now = SystemTime::now() .duration_since(UNIX_EPOCH) .expect("system time before Unix epoch"); - let naive = - NaiveDateTime::from_timestamp_opt(now.as_secs() as i64, now.subsec_nanos()).unwrap(); + let naive = DateTime::from_timestamp(now.as_secs() as i64, now.subsec_nanos()) + .expect("DateTime::from_timestamp") + .naive_utc(); Utc.from_utc_datetime(&naive) }