Suppress FileNotFoundError when deleting keys in the obstore adapter (#3140)

* Suppress FileNotFoundError when deleting keys in the obstore adapter

* Add unit test

* Document bugfix changes

* Run delete nonexistent key test across all stores

* Suppress only builtins.FileNotFoundError in obstore.delete adapter

---------

Co-authored-by: Davis Bennett <davis.v.bennett@gmail.com>

Author: lukasbindreiter

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.

src/zarr/storage/_obstore.py

@@ -188,7 +188,13 @@ async def delete(self, key: str) -> None:
         import obstore as obs
 
         self._check_writable()
-        await obs.delete_async(self.store, key)
+
+        # 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 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
     def supports_partial_writes(self) -> bool:

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(