From 120a6bc6065dd7c347f71da50968530fe07edd80 Mon Sep 17 00:00:00 2001 From: Lukas Bindreiter Date: Wed, 18 Jun 2025 10:03:10 +0200 Subject: [PATCH 1/5] Suppress FileNotFoundError when deleting keys in the obstore adapter --- src/zarr/storage/_obstore.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/zarr/storage/_obstore.py b/src/zarr/storage/_obstore.py index 047ed07fbb..dc89c76454 100644 --- a/src/zarr/storage/_obstore.py +++ b/src/zarr/storage/_obstore.py @@ -188,7 +188,12 @@ async def delete(self, key: str) -> None: import obstore as obs self._check_writable() - await obs.delete_async(self.store, key) + + # Some stores such as local filesystems, GCP and Azure raise an error + # when deleting a non-existent key, while others such as S3 and in-memory do + # not. We suppress the error to make the behavior consistent across all stores + with contextlib.suppress(FileNotFoundError): + await obs.delete_async(self.store, key) @property def supports_partial_writes(self) -> bool: From c6ded01105eb06013bfd8af45857bb7605152cc8 Mon Sep 17 00:00:00 2001 From: Lukas Bindreiter Date: Wed, 18 Jun 2025 10:22:37 +0200 Subject: [PATCH 2/5] Add unit test --- tests/test_store/test_object.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/tests/test_store/test_object.py b/tests/test_store/test_object.py index 4d9e8fcc1f..25d0623b35 100644 --- a/tests/test_store/test_object.py +++ b/tests/test_store/test_object.py @@ -75,6 +75,9 @@ def test_store_init_raises(self) -> None: with pytest.raises(TypeError): ObjectStore("path/to/store") + async def test_store_delete_nonexistent_key_does_not_raise(self, store: ObjectStore) -> None: + await store.delete("nonexistent_key") + @pytest.mark.slow_hypothesis def test_zarr_hierarchy(): From 90c6c024817c9e8773d3a000c17165921fb3fd10 Mon Sep 17 00:00:00 2001 From: Lukas Bindreiter Date: Wed, 18 Jun 2025 10:36:15 +0200 Subject: [PATCH 3/5] Document bugfix changes --- changes/3140.bugfix.rst | 8 ++++++++ 1 file changed, 8 insertions(+) create mode 100644 changes/3140.bugfix.rst diff --git a/changes/3140.bugfix.rst b/changes/3140.bugfix.rst new file mode 100644 index 0000000000..6ef83c90a5 --- /dev/null +++ b/changes/3140.bugfix.rst @@ -0,0 +1,8 @@ +Suppress `FileNotFoundError` when deleting non-existent keys in the `obstore` adapter. + +When writing empty chunks (i.e. chunks where all values are equal to the array's fill value) to a zarr array, zarr +will delete those chunks from the underlying store. For zarr arrays backed by the `obstore` adapter, this will potentially +raise a `FileNotFoundError` if the chunk doesn't already exist. +Since whether or not a delete of a non-existing object raises an error depends on the behavior of the underlying store, +suppressing the error in all cases results in consistent behavior across stores, and is also what `zarr` seems to expect +from the store. From e7400273e6673aeb0522cf20a82e56d8622f0d2e Mon Sep 17 00:00:00 2001 From: Lukas Bindreiter Date: Mon, 23 Jun 2025 10:25:51 +0200 Subject: [PATCH 4/5] Run delete nonexistent key test across all stores --- src/zarr/storage/_obstore.py | 5 ++++- src/zarr/testing/store.py | 5 +++++ tests/test_store/test_object.py | 3 --- 3 files changed, 9 insertions(+), 4 deletions(-) diff --git a/src/zarr/storage/_obstore.py b/src/zarr/storage/_obstore.py index dc89c76454..9389a74895 100644 --- a/src/zarr/storage/_obstore.py +++ b/src/zarr/storage/_obstore.py @@ -186,13 +186,16 @@ def supports_deletes(self) -> bool: async def delete(self, key: str) -> None: # docstring inherited import obstore as obs + from obstore.exceptions import NotFoundError self._check_writable() # Some stores such as local filesystems, GCP and Azure raise an error # when deleting a non-existent key, while others such as S3 and in-memory do # not. We suppress the error to make the behavior consistent across all stores - with contextlib.suppress(FileNotFoundError): + # currently obstore raises FileNotFoundError, but in the future might raise + # NotFoundError instead, so let's suppress that too + with contextlib.suppress(FileNotFoundError, NotFoundError): await obs.delete_async(self.store, key) @property diff --git a/src/zarr/testing/store.py b/src/zarr/testing/store.py index 970329f393..d2946705f0 100644 --- a/src/zarr/testing/store.py +++ b/src/zarr/testing/store.py @@ -401,6 +401,11 @@ async def test_delete_dir(self, store: S) -> None: assert not await store.exists("foo/zarr.json") assert not await store.exists("foo/c/0") + async def test_delete_nonexistent_key_does_not_raise(self, store: S) -> None: + if not store.supports_deletes: + pytest.skip("store does not support deletes") + await store.delete("nonexistent_key") + async def test_is_empty(self, store: S) -> None: assert await store.is_empty("") await self.set( diff --git a/tests/test_store/test_object.py b/tests/test_store/test_object.py index 25d0623b35..4d9e8fcc1f 100644 --- a/tests/test_store/test_object.py +++ b/tests/test_store/test_object.py @@ -75,9 +75,6 @@ def test_store_init_raises(self) -> None: with pytest.raises(TypeError): ObjectStore("path/to/store") - async def test_store_delete_nonexistent_key_does_not_raise(self, store: ObjectStore) -> None: - await store.delete("nonexistent_key") - @pytest.mark.slow_hypothesis def test_zarr_hierarchy(): From 76a04977e541308e0973b464e522dacc9264bf69 Mon Sep 17 00:00:00 2001 From: Lukas Bindreiter Date: Tue, 24 Jun 2025 10:03:37 +0200 Subject: [PATCH 5/5] Suppress only builtins.FileNotFoundError in obstore.delete adapter --- src/zarr/storage/_obstore.py | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/src/zarr/storage/_obstore.py b/src/zarr/storage/_obstore.py index 9389a74895..1b822a919e 100644 --- a/src/zarr/storage/_obstore.py +++ b/src/zarr/storage/_obstore.py @@ -186,16 +186,14 @@ def supports_deletes(self) -> bool: async def delete(self, key: str) -> None: # docstring inherited import obstore as obs - from obstore.exceptions import NotFoundError self._check_writable() - # Some stores such as local filesystems, GCP and Azure raise an error + # Some obstore stores such as local filesystems, GCP and Azure raise an error # when deleting a non-existent key, while others such as S3 and in-memory do - # not. We suppress the error to make the behavior consistent across all stores - # currently obstore raises FileNotFoundError, but in the future might raise - # NotFoundError instead, so let's suppress that too - with contextlib.suppress(FileNotFoundError, NotFoundError): + # not. We suppress the error to make the behavior consistent across all obstore + # stores. This is also in line with the behavior of the other Zarr store adapters. + with contextlib.suppress(FileNotFoundError): await obs.delete_async(self.store, key) @property