Skip to content

fix: handle ObjectStore.list_dir when prefix-like object exists#4038

Open
NIK-TIGER-BILL wants to merge 2 commits into
zarr-developers:mainfrom
NIK-TIGER-BILL:fix-list-dir
Open

fix: handle ObjectStore.list_dir when prefix-like object exists#4038
NIK-TIGER-BILL wants to merge 2 commits into
zarr-developers:mainfrom
NIK-TIGER-BILL:fix-list-dir

Conversation

@NIK-TIGER-BILL
Copy link
Copy Markdown
Contributor

This PR fixes #4032 by filtering out entries that exactly match the prefix (or the prefix with a trailing slash) in _transform_list_dir.

Previously, if an object keyed exactly like the prefix existed (e.g., "g/" when listing "g"), _relativize_path would either raise ValueError or yield an empty string, because it expects paths to start with prefix + "/".

Changes

  • In _transform_list_dir, skip entries where path == prefix or path == prefix + "/".
  • Skip empty-string results after relativization.

Tests

  • Added test_list_dir_prefix_object to verify that list_dir("g") does not raise when an object keyed "g/" exists.

PR Checklist

  • I have read the contributing guide.
  • I have written tests for my changes.
  • My changes generate no new warnings.
  • I have signed all my commits with DCO (git commit -s).

When an object keyed exactly like the prefix (e.g., "g/" when prefix is "g")
exists, list_dir would raise ValueError or yield an empty string because
_relativize_path expects the path to start with prefix + "/".

This commit filters out entries that exactly match the prefix or are the
prefix with a trailing slash, preventing both the error and the empty string
yield.

Fixes zarr-developers#4032

Signed-off-by: NIK-TIGER-BILL <nik.tiger.bill@github.com>
@github-actions github-actions Bot added the needs release notes Automatically applied to PRs which haven't added release notes label Jun 5, 2026
@codecov
Copy link
Copy Markdown

codecov Bot commented Jun 5, 2026

Codecov Report

❌ Patch coverage is 66.66667% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 93.52%. Comparing base (fe22910) to head (d4d9f6b).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
src/zarr/storage/_obstore.py 66.66% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #4038      +/-   ##
==========================================
- Coverage   93.55%   93.52%   -0.04%     
==========================================
  Files          88       88              
  Lines       11896    11899       +3     
==========================================
- Hits        11129    11128       -1     
- Misses        767      771       +4     
Files with missing lines Coverage Δ
src/zarr/storage/_obstore.py 93.72% <66.66%> (-0.78%) ⬇️

... and 1 file with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

await self.set(store, "g/", buf)
# list_dir("g") should not raise even though "g/" looks like the prefix itself.
result = [k async for k in store.list_dir("g")]
# The entry should not appear as an empty string or raise.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we instead assert that result is exactly what we expect, instead of asserting that something we don't expect is not in result?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point — changed to assert result == [] to assert the exact expected result instead of a negative check. Thanks for the feedback!

…r test

Signed-off-by: NIK-TIGER-BILL <nik.tiger.bill@github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

needs release notes Automatically applied to PRs which haven't added release notes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ObjectStore.list_dir("g") raises ValueError when an object keyed "g/" exists

2 participants