From ef7eb0f6ceedd91590499059e2947d8f395f5a36 Mon Sep 17 00:00:00 2001 From: mayuriphad Date: Sat, 3 Oct 2026 16:48:14 +0530 Subject: [PATCH 1/3] Exclude in-progress atomic-write temp files from LocalStore listings LocalStore.list, list_prefix and list_dir returned the '..partial' files created by _atomic_write while a write was in progress. Readers then treated them as store keys and emitted ZarrUserWarning for each one. Filter files matching the temp-file pattern in _list_files and _list_dir and add a regression test. Fixes #4161. --- changes/4161.bugfix.md | 1 + src/zarr/storage/_local.py | 17 +++++++++++++++-- tests/test_store/test_local.py | 14 ++++++++++++++ 3 files changed, 30 insertions(+), 2 deletions(-) create mode 100644 changes/4161.bugfix.md diff --git a/changes/4161.bugfix.md b/changes/4161.bugfix.md new file mode 100644 index 0000000000..92c7b2550c --- /dev/null +++ b/changes/4161.bugfix.md @@ -0,0 +1 @@ +`LocalStore.list()`, `list_prefix()` and `list_dir()` no longer include the temporary `.partial` files created while a write is in progress, which previously caused spurious `ZarrUserWarning`s when reading a store that another process was writing to. diff --git a/src/zarr/storage/_local.py b/src/zarr/storage/_local.py index 20d4b102b1..f5b8388a43 100644 --- a/src/zarr/storage/_local.py +++ b/src/zarr/storage/_local.py @@ -4,6 +4,7 @@ import contextlib import io import os +import re import shutil import sys import time @@ -171,15 +172,27 @@ def _getsize(path: Path) -> int: return path.stat().st_size +_PARTIAL_SUFFIX_RE = re.compile(r"\.[0-9a-f]{32}\.partial$") + + +def _is_partial_temp(path: Path) -> bool: + """Whether `path` is a temporary file created by `_atomic_write` (never a store key).""" + return _PARTIAL_SUFFIX_RE.search(path.name) is not None + + def _list_files(root: Path, prefix: str) -> list[str]: """Keys (paths relative to `root`, POSIX style) of every file under `root / prefix`.""" to_strip = root.as_posix() + "/" - return [p.as_posix().removeprefix(to_strip) for p in (root / prefix).rglob("*") if p.is_file()] + return [ + p.as_posix().removeprefix(to_strip) + for p in (root / prefix).rglob("*") + if p.is_file() and not _is_partial_temp(p) + ] def _list_dir(base: Path) -> list[str]: try: - return [p.name for p in base.iterdir()] + return [p.name for p in base.iterdir() if not _is_partial_temp(p)] except (FileNotFoundError, NotADirectoryError): return [] diff --git a/tests/test_store/test_local.py b/tests/test_store/test_local.py index c619177c34..a1bbb13849 100644 --- a/tests/test_store/test_local.py +++ b/tests/test_store/test_local.py @@ -461,3 +461,17 @@ def test_atomic_write_onto_directory( f.write(b"abc") assert path.is_dir() assert list(tmp_path.iterdir()) == [path] # no temp files + + +async def test_list_excludes_partial_temp_files(tmp_path: pathlib.Path) -> None: + """In-flight `_atomic_write` temp files are not store keys and must not be listed.""" + store = LocalStore(tmp_path) + await store.set("group/array/c/0", cpu.Buffer.from_bytes(b"data")) + # Simulate a concurrent writer that has not yet renamed its temp file into place. + temp = tmp_path / "group" / "array" / "zarr.0123456789abcdef0123456789abcdef.partial" + temp.write_bytes(b"x") + + keys = [key async for key in store.list()] + assert keys == ["group/array/c/0"] + assert [key async for key in store.list_prefix("group")] == ["group/array/c/0"] + assert sorted([name async for name in store.list_dir("group/array")]) == ["c"] From 7a82dc8bf7bc177ac1887b6c0dd072a468890d83 Mon Sep 17 00:00:00 2001 From: mayuriphad Date: Sun, 4 Oct 2026 08:28:05 +0530 Subject: [PATCH 2/3] Skip non-hierarchy objects silently in member listing instead of filtering LocalStore Per review: keep LocalStore listings faithful to what is on disk (including in-progress .partial temp files) and drop the ZarrUserWarning emitted when group member iteration encounters an object that is not a Zarr node. --- changes/4161.bugfix.md | 2 +- src/zarr/core/group.py | 8 ++------ src/zarr/storage/_local.py | 17 ++--------------- tests/test_group.py | 30 +++++++++++++++++------------- tests/test_store/test_local.py | 14 -------------- 5 files changed, 22 insertions(+), 49 deletions(-) diff --git a/changes/4161.bugfix.md b/changes/4161.bugfix.md index 92c7b2550c..98fb052825 100644 --- a/changes/4161.bugfix.md +++ b/changes/4161.bugfix.md @@ -1 +1 @@ -`LocalStore.list()`, `list_prefix()` and `list_dir()` no longer include the temporary `.partial` files created while a write is in progress, which previously caused spurious `ZarrUserWarning`s when reading a store that another process was writing to. +Listing the members of a group no longer emits a `ZarrUserWarning` for store objects that are not part of the Zarr hierarchy (such as the temporary `.partial` files created while a write is in progress); these objects are now skipped silently. diff --git a/src/zarr/core/group.py b/src/zarr/core/group.py index 7792fa58de..29d802df8e 100644 --- a/src/zarr/core/group.py +++ b/src/zarr/core/group.py @@ -3602,15 +3602,11 @@ async def _iter_members( for fetched_node_coro in asyncio.as_completed(node_tasks): try: fetched_node = await fetched_node_coro - except KeyError as e: + except KeyError: # keyerror is raised when `key` names an object (in the object storage sense), # as opposed to a prefix, in the store under the prefix associated with this group # in which case `key` cannot be the name of a sub-array or sub-group. - warnings.warn( - f"Object at {e.args[0]} is not recognized as a component of a Zarr hierarchy.", - ZarrUserWarning, - stacklevel=1, - ) + # Such objects (e.g. temporary files from an in-progress write) are skipped silently. continue match fetched_node: case AsyncArray() | AsyncGroup(): diff --git a/src/zarr/storage/_local.py b/src/zarr/storage/_local.py index f5b8388a43..20d4b102b1 100644 --- a/src/zarr/storage/_local.py +++ b/src/zarr/storage/_local.py @@ -4,7 +4,6 @@ import contextlib import io import os -import re import shutil import sys import time @@ -172,27 +171,15 @@ def _getsize(path: Path) -> int: return path.stat().st_size -_PARTIAL_SUFFIX_RE = re.compile(r"\.[0-9a-f]{32}\.partial$") - - -def _is_partial_temp(path: Path) -> bool: - """Whether `path` is a temporary file created by `_atomic_write` (never a store key).""" - return _PARTIAL_SUFFIX_RE.search(path.name) is not None - - def _list_files(root: Path, prefix: str) -> list[str]: """Keys (paths relative to `root`, POSIX style) of every file under `root / prefix`.""" to_strip = root.as_posix() + "/" - return [ - p.as_posix().removeprefix(to_strip) - for p in (root / prefix).rglob("*") - if p.is_file() and not _is_partial_temp(p) - ] + return [p.as_posix().removeprefix(to_strip) for p in (root / prefix).rglob("*") if p.is_file()] def _list_dir(base: Path) -> list[str]: try: - return [p.name for p in base.iterdir() if not _is_partial_temp(p)] + return [p.name for p in base.iterdir()] except (FileNotFoundError, NotADirectoryError): return [] diff --git a/tests/test_group.py b/tests/test_group.py index 6976ef536c..ea02f7a5c2 100644 --- a/tests/test_group.py +++ b/tests/test_group.py @@ -217,38 +217,42 @@ def test_group_members(store: Store, zarr_format: ZarrFormat, consolidated_metad ) ) - # this warning shows up when extra objects show up in the hierarchy - warn_context = pytest.warns( - ZarrUserWarning, - match=r"(?:Object at .* is not recognized as a component of a Zarr hierarchy.)|(?:Consolidated metadata is currently not part in the Zarr format 3 specification.)", - ) if consolidated_metadata: + consolidate_warn_context = ( + pytest.warns( + ZarrUserWarning, + match="Consolidated metadata is currently not part in the Zarr format 3 specification.", + ) + if zarr_format == 3 + else contextlib.nullcontext() + ) if isinstance(store, ZipStore): - with warn_context: + with consolidate_warn_context: with pytest.warns(UserWarning, match="Duplicate name: "): zarr.consolidate_metadata(store=store, zarr_format=zarr_format) else: - with warn_context: + with consolidate_warn_context: zarr.consolidate_metadata(store=store, zarr_format=zarr_format) - # now that we've consolidated the store, we shouldn't get the warnings from the unrecognized objects anymore - # we use a nullcontext to handle these cases - warn_context = contextlib.nullcontext() group = zarr.open_consolidated(store=store, zarr_format=zarr_format) - with warn_context: + # extra objects in the hierarchy are skipped without warning + with warnings.catch_warnings(): + warnings.simplefilter("error", ZarrUserWarning) members_observed = group.members() # members are not guaranteed to be ordered, so sort before comparing assert sorted(dict(members_observed)) == sorted(members_expected) # partial - with warn_context: + with warnings.catch_warnings(): + warnings.simplefilter("error", ZarrUserWarning) members_observed = group.members(max_depth=1) members_expected["subgroup/subsubgroup"] = subsubgroup # members are not guaranteed to be ordered, so sort before comparing assert sorted(dict(members_observed)) == sorted(members_expected) # total - with warn_context: + with warnings.catch_warnings(): + warnings.simplefilter("error", ZarrUserWarning) members_observed = group.members(max_depth=None) members_expected["subgroup/subsubgroup/subsubsubgroup"] = subsubsubgroup # members are not guaranteed to be ordered, so sort before comparing diff --git a/tests/test_store/test_local.py b/tests/test_store/test_local.py index a1bbb13849..c619177c34 100644 --- a/tests/test_store/test_local.py +++ b/tests/test_store/test_local.py @@ -461,17 +461,3 @@ def test_atomic_write_onto_directory( f.write(b"abc") assert path.is_dir() assert list(tmp_path.iterdir()) == [path] # no temp files - - -async def test_list_excludes_partial_temp_files(tmp_path: pathlib.Path) -> None: - """In-flight `_atomic_write` temp files are not store keys and must not be listed.""" - store = LocalStore(tmp_path) - await store.set("group/array/c/0", cpu.Buffer.from_bytes(b"data")) - # Simulate a concurrent writer that has not yet renamed its temp file into place. - temp = tmp_path / "group" / "array" / "zarr.0123456789abcdef0123456789abcdef.partial" - temp.write_bytes(b"x") - - keys = [key async for key in store.list()] - assert keys == ["group/array/c/0"] - assert [key async for key in store.list_prefix("group")] == ["group/array/c/0"] - assert sorted([name async for name in store.list_dir("group/array")]) == ["c"] From 7039c5631988240ae73974a6b175be6262bd745c Mon Sep 17 00:00:00 2001 From: mayuriphad Date: Sun, 4 Oct 2026 10:59:18 +0530 Subject: [PATCH 3/3] Update consolidated members test: unknown children are now skipped silently --- tests/test_metadata/test_consolidated.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/tests/test_metadata/test_consolidated.py b/tests/test_metadata/test_consolidated.py index cd0fd92d74..685c481e77 100644 --- a/tests/test_metadata/test_consolidated.py +++ b/tests/test_metadata/test_consolidated.py @@ -747,8 +747,7 @@ async def test_use_consolidated_for_children_members( # Now according to the consolidated metadata, "a" has children ["b"] # but according to the unconsolidated metadata, "a" has children ["b", "c"] group = await zarr.api.asynchronous.open_group(store=memory_store, path="a") - with pytest.warns(ZarrUserWarning, match="Object at 'c' not found"): - result = sorted([x[0] async for x in group.members(max_depth=None)]) + result = sorted([x[0] async for x in group.members(max_depth=None)]) expected = ["b"] assert result == expected