Skip to content

Commit 1b6a1da

Browse files
authored
Fix config serialization (#1187)
* Fix config serialization We were failing to deserialize in certain situations when there are virtual chunk containers. This change breaks compatibility for users who wrote config with virtual chunk containers to their repos. Those users will need to edit the config.yaml file adding ``` name: null ``` to their virtual chunk containers. Closes: #1181 * Allow missing name field for VirtualChunkContainers * Add pickle/unpickle to stateful test
1 parent cf9b97f commit 1b6a1da

File tree

4 files changed

+369
-33
lines changed

4 files changed

+369
-33
lines changed

icechunk-python/tests/test_zarr/test_stateful.py

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import functools
22
import json
3+
import pickle
34
from typing import Any
45

56
import hypothesis.extra.numpy as npst
@@ -345,6 +346,14 @@ def delete_dir(self, data) -> None:
345346
self.all_groups = self.all_groups - matches
346347
self.all_arrays = self.all_arrays - matches
347348

349+
@rule()
350+
def pickle_objects(self) -> None:
351+
if not self.store.session.has_uncommitted_changes:
352+
session = self.store.session.fork()
353+
pickle.loads(pickle.dumps(session))
354+
355+
pickle.loads(pickle.dumps(self.repo))
356+
348357
@invariant()
349358
def check_list_prefix_from_root(self) -> None:
350359
model_list = self._sync_iter(self.model.list_prefix(""))

icechunk/src/config.rs

Lines changed: 84 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -587,3 +587,87 @@ pub enum Credentials {
587587
Gcs(GcsCredentials),
588588
Azure(AzureCredentials),
589589
}
590+
591+
#[cfg(test)]
592+
#[allow(clippy::panic, clippy::unwrap_used, clippy::expect_used)]
593+
mod tests {
594+
use crate::{
595+
ObjectStoreConfig, RepositoryConfig, config::S3Options,
596+
strategies::repository_config, virtual_chunks::VirtualChunkContainer,
597+
};
598+
599+
use proptest::prelude::*;
600+
601+
#[icechunk_macros::test]
602+
fn test_config_serialization() {
603+
let mut config = RepositoryConfig::default();
604+
let bytes = rmp_serde::to_vec(&config).unwrap();
605+
let roundtrip = rmp_serde::from_slice(&bytes).unwrap();
606+
assert_eq!(config, roundtrip);
607+
608+
config.set_virtual_chunk_container(
609+
VirtualChunkContainer::new(
610+
"s3://foo/bar/".to_string(),
611+
ObjectStoreConfig::S3(S3Options {
612+
region: Some("us-east-1".to_string()),
613+
endpoint_url: None,
614+
anonymous: false,
615+
allow_http: false,
616+
force_path_style: false,
617+
network_stream_timeout_seconds: None,
618+
}),
619+
)
620+
.unwrap(),
621+
);
622+
let bytes = rmp_serde::to_vec(&config).unwrap();
623+
let roundtrip = rmp_serde::from_slice(&bytes).unwrap();
624+
assert_eq!(config, roundtrip);
625+
}
626+
627+
#[icechunk_macros::test]
628+
fn test_virtual_container_serialization() {
629+
let container = VirtualChunkContainer::new(
630+
"s3://foo/bar/".to_string(),
631+
ObjectStoreConfig::S3(S3Options {
632+
region: Some("us-east-1".to_string()),
633+
endpoint_url: None,
634+
anonymous: false,
635+
allow_http: false,
636+
force_path_style: false,
637+
network_stream_timeout_seconds: None,
638+
}),
639+
)
640+
.unwrap();
641+
let bytes = rmp_serde::to_vec(&container).unwrap();
642+
let roundtrip = rmp_serde::from_slice(&bytes).unwrap();
643+
assert_eq!(container, roundtrip);
644+
}
645+
646+
#[icechunk_macros::test]
647+
fn test_object_store_config_serialization() {
648+
let config = ObjectStoreConfig::S3(S3Options {
649+
region: Some("us-east-1".to_string()),
650+
endpoint_url: None,
651+
anonymous: false,
652+
allow_http: false,
653+
force_path_style: false,
654+
network_stream_timeout_seconds: None,
655+
});
656+
let bytes = rmp_serde::to_vec(&config).unwrap();
657+
let roundtrip = rmp_serde::from_slice(&bytes).unwrap();
658+
assert_eq!(config, roundtrip);
659+
}
660+
661+
proptest! {
662+
#![proptest_config(ProptestConfig {
663+
cases: 50, .. ProptestConfig::default()
664+
})]
665+
666+
#[icechunk_macros::test]
667+
fn test_config_roundtrip(config in repository_config() ) {
668+
let bytes = rmp_serde::to_vec(&config).unwrap();
669+
let roundtrip = rmp_serde::from_slice(&bytes).unwrap();
670+
assert_eq!(config, roundtrip);
671+
}
672+
}
673+
}

0 commit comments

Comments
 (0)