Skip to content

Commit e03cfc8

Browse files
antoinegastond-v-b
andauthored
🐛 Fix obstore listdir method using _relativize_path (#3657)
* 🐛 Fix obstore listdir method using _relativize_path * 📝 Add changelog entry * its a bugfix --------- Co-authored-by: Davis Bennett <davis.v.bennett@gmail.com>
1 parent a06318e commit e03cfc8

File tree

3 files changed

+33
-16
lines changed

3 files changed

+33
-16
lines changed

changes/3657.bugfix.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
Fix obstore _transform_list_dir implementation to correctly relativize paths (removing lstrip usage).

src/zarr/storage/_obstore.py

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,8 @@
44
import contextlib
55
import pickle
66
from collections import defaultdict
7+
from itertools import chain
8+
from operator import itemgetter
79
from typing import TYPE_CHECKING, Generic, Self, TypedDict, TypeVar
810

911
from zarr.abc.store import (
@@ -15,6 +17,7 @@
1517
)
1618
from zarr.core.common import concurrent_map
1719
from zarr.core.config import config
20+
from zarr.storage._utils import _relativize_path
1821

1922
if TYPE_CHECKING:
2023
from collections.abc import AsyncGenerator, Coroutine, Iterable, Sequence
@@ -263,10 +266,11 @@ async def _transform_list_dir(
263266
# We assume that the underlying object-store implementation correctly handles the
264267
# prefix, so we don't double-check that the returned results actually start with the
265268
# given prefix.
266-
prefixes = [obj.lstrip(prefix).lstrip("/") for obj in list_result["common_prefixes"]]
267-
objects = [obj["path"].removeprefix(prefix).lstrip("/") for obj in list_result["objects"]]
268-
for item in prefixes + objects:
269-
yield item
269+
prefix = prefix.rstrip("/")
270+
for path in chain(
271+
list_result["common_prefixes"], map(itemgetter("path"), list_result["objects"])
272+
):
273+
yield _relativize_path(path=path, prefix=prefix)
270274

271275

272276
class _BoundedRequest(TypedDict):

src/zarr/testing/store.py

Lines changed: 24 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -492,24 +492,36 @@ async def test_list_empty_path(self, store: S) -> None:
492492
assert observed_prefix_sorted == expected_prefix_sorted
493493

494494
async def test_list_dir(self, store: S) -> None:
495-
root = "foo"
496-
store_dict = {
497-
root + "/zarr.json": self.buffer_cls.from_bytes(b"bar"),
498-
root + "/c/1": self.buffer_cls.from_bytes(b"\x01"),
499-
}
495+
roots_and_keys: list[tuple[str, dict[str, Buffer]]] = [
496+
(
497+
"foo",
498+
{
499+
"foo/zarr.json": self.buffer_cls.from_bytes(b"bar"),
500+
"foo/c/1": self.buffer_cls.from_bytes(b"\x01"),
501+
},
502+
),
503+
(
504+
"foo/bar",
505+
{
506+
"foo/bar/foobar_first_child": self.buffer_cls.from_bytes(b"1"),
507+
"foo/bar/foobar_second_child/zarr.json": self.buffer_cls.from_bytes(b"2"),
508+
},
509+
),
510+
]
500511

501512
assert await _collect_aiterator(store.list_dir("")) == ()
502-
assert await _collect_aiterator(store.list_dir(root)) == ()
503513

504-
await store._set_many(store_dict.items())
514+
for root, store_dict in roots_and_keys:
515+
assert await _collect_aiterator(store.list_dir(root)) == ()
505516

506-
keys_observed = await _collect_aiterator(store.list_dir(root))
507-
keys_expected = {k.removeprefix(root + "/").split("/")[0] for k in store_dict}
517+
await store._set_many(store_dict.items())
508518

509-
assert sorted(keys_observed) == sorted(keys_expected)
519+
keys_observed = await _collect_aiterator(store.list_dir(root))
520+
keys_expected = {k.removeprefix(root + "/").split("/")[0] for k in store_dict}
521+
assert sorted(keys_observed) == sorted(keys_expected)
510522

511-
keys_observed = await _collect_aiterator(store.list_dir(root + "/"))
512-
assert sorted(keys_expected) == sorted(keys_observed)
523+
keys_observed = await _collect_aiterator(store.list_dir(root + "/"))
524+
assert sorted(keys_expected) == sorted(keys_observed)
513525

514526
async def test_set_if_not_exists(self, store: S) -> None:
515527
key = "k"

0 commit comments

Comments
 (0)