Skip to content

Commit

Permalink
Fix stateful repo ops test (#546)
Browse files Browse the repository at this point in the history
  • Loading branch information
dcherian authored Jan 6, 2025
1 parent 6d9c9df commit fd8f7c2
Show file tree
Hide file tree
Showing 2 changed files with 18 additions and 3 deletions.
15 changes: 12 additions & 3 deletions icechunk-python/tests/test_stateful_repo_ops.py
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ def v3_group_metadata(draw):


@st.composite
def v3_array_metadata(draw):
def v3_array_metadata(draw: st.DrawFn) -> bytes:
from zarr.codecs.bytes import BytesCodec
from zarr.core.chunk_grids import RegularChunkGrid
from zarr.core.chunk_key_encodings import DefaultChunkKeyEncoding
Expand Down Expand Up @@ -127,11 +127,20 @@ def __repr__(self) -> str:
Branches: {tuple(self.branches.keys())!r}
Tags: {tuple(self.tags.keys())!r}""").strip("\n")

def __setitem__(self, key, value):
def __setitem__(self, key: str, value: Buffer) -> None:
# Icechunk doesn't overwrite docs with the same value
# and we need to keep `changes_made` in sync.
# Icechunk checks after decoding the metadata to rust Structs so we must do the same.
# Different byte strings can decode to the same json dict (order of user attributes may be different)
if key in self.store and json.loads(self.store[key].to_bytes()) == json.loads(
value.to_bytes()
):
note(f"skipping setting {key!r}, value is unchanged")
return
self.changes_made = True
self.store[key] = value

def __getitem__(self, key):
def __getitem__(self, key: str) -> Buffer:
return self.store[key]

@property
Expand Down
6 changes: 6 additions & 0 deletions icechunk/src/store.rs
Original file line number Diff line number Diff line change
Expand Up @@ -654,6 +654,12 @@ async fn set_array_meta(
array_meta: ArrayMetadata,
repo: &mut Session,
) -> StoreResult<()> {
// TODO: Consider deleting all this logic?
// We try hard to not overwrite existing metadata here.
// I don't think this is actually useful because Zarr's
// Array/Group API will require that the user set `overwrite=True`
// which will delete any existing array metadata. This path is only
// applicable when using the explicit `store.set` interface.
if let Ok(node) = repo.get_array(&path).await {
// Check if the user attributes are different, if they are update them
let existing_attrs = match node.user_attributes {
Expand Down

0 comments on commit fd8f7c2

Please sign in to comment.