feat: url-pipeline core — parser, adapter ABC, registry, store hooks

[jhamman]

Summary

This PR delivers the core infrastructure for plugable URL pipeline parsing.

Implements URL pipeline support (https://github.com/jbms/url-pipeline): '|'-chained URLs are resolves through pluggable adapters registered under the 'zarr.url_adapters' entry-point group (entry-point name = URL scheme).

• zarr.abc.url_pipeline: PipelineSegment, AdapterResolution, PipelineContext, URLPipelineAdapter (single-classmethod contract)
• zarr.storage._url_pipeline: parse_pipeline / resolve_pipeline; the root sub-URL delegates to make_store so existing file/memory/fsspec routing is unchanged
• registry: register_url_adapter / get_url_adapter / list_url_adapter_schemes (name check only; no adapter imports)
• make_store/make_store_path route strings containing '|' (or a registered root scheme) through the resolver; residual store paths combine with the user-supplied path
• StorePath gains a zarr_format attribute (populated by format segments in a follow-up)

For reviewers

This is PR 2 in a series towards #2943

Author attestation

• I am a human, these are my changes, and I have reviewed and understood every change and can explain why each is correct.

TODO

• Add unit tests and/or doctests in docstrings
• Add docstrings and API docs for any new/modified user-facing classes and functions
• New/modified features documented in docs/user-guide/*.md
• Changes documented as a new file in changes/
• GitHub Actions have all passed
• Test coverage is 100% (Codecov passes)

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 94.21%. Comparing base (ce10c0b) to head (20f2ac8).

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #4192      +/-   ##
==========================================
+ Coverage   94.12%   94.21%   +0.09%     
==========================================
  Files          92       94       +2     
  Lines       12830    13041     +211     
==========================================
+ Hits        12076    12287     +211     
  Misses        754      754              

Files with missing lines	Coverage Δ
src/zarr/abc/url_pipeline.py	100.00% <100.00%> (ø)
src/zarr/api/asynchronous.py	96.49% <100.00%> (+0.16%)	⬆️
src/zarr/errors.py	100.00% <100.00%> (ø)
src/zarr/registry.py	91.06% <100.00%> (+1.58%)	⬆️
src/zarr/storage/__init__.py	95.23% <100.00%> (+0.23%)	⬆️
src/zarr/storage/_common.py	93.66% <100.00%> (+0.52%)	⬆️
src/zarr/storage/_url_pipeline.py	100.00% <100.00%> (ø)

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[d-v-b]

can we get property-based tests that create valid pipelines up to depth 8 or so, sampled from all valid root and adapter schemes, and ensure that these pipelines comply with the invariants tested for a few examples here? And if there's a convenient way to generate invalid pipelines, that would also be nice for property testing

[d-v-b]

This is great! Here's some stuff I found with Claude:

🤖 AI text below 🤖

Review of c744f95. The parser and adapter ABC look solid (the spec corpus passes, ruff/mypy are clean, no import cycles, test order doesn't leak registry state), but the store hooks introduce some regressions and there are a few contract questions worth settling before the zip:/zarr3: follow-ups build on this. Everything below was reproduced locally unless marked otherwise.

Correctness / regressions

1. | is now reserved in every string store spec, with no escape, and str vs Path diverge (_url_pipeline.py:115). Any string containing | is routed to the pipeline machinery:

   zarr.open_group(f"{tmp}/a|b", mode="w")        # main: LocalStore. PR: URLPipelineError: no URL pipeline adapter is registered for scheme 'b'
   zarr.open_group(Path(f"{tmp}/a|b"), mode="w")  # still works

   Reserving | is presumably intended (it's the spec delimiter), but the changelog only says URLs without | are unchanged; it should say | is now reserved — and there's currently no escape, since %7C is not decoded for local paths (open_group(f"{tmp}/a%7Cb") roots at a literal a%7Cb). Same story for #: the _split_query error tells users to write %23, but nothing decodes it, so a local file with # or | in its name is unreachable through a pipeline in either spelling.

2. Local file roots can't be resolved by wrapper adapters except in mode "r" (_url_pipeline.py:161). The base case hands the root to make_store, which builds a LocalStore and mkdirs it (_local.py:169) for every mode but "r". With a minimal adapter that returns context.resolve_preceding():

   await make_store_path(f"{tmp}/data.zip|wrap:")   # mode None / "a" / "w" / "r+" / "w-" -> FileExistsError: [Errno 17] File exists: '.../data.zip'

   In "r" the wrapper gets a LocalStore rooted at the file, whose only useful op is the accidental get(""). And for a not-yet-existing archive, zarr.open("new.zip|zip:", mode="w") creates a directory named new.zip before the adapter runs. There's also no way for an adapter to ask for the root to be opened read-only/non-creating (resolve_preceding() takes no mode). The base case needs a file-vs-directory notion (or wrapper adapters need a different primitive) before the zip PR.

3. make_store(url, mode="r") returns a writable store when the adapter ignores context.read_only (_common.py:357-364). Every other make_store branch bakes read_only = mode == "r" into the constructor; the pipeline branch returns result.store untouched. make_store_path gets it right only incidentally via StorePath.open(mode="r") → with_read_only(True). The CLI (_cli/cli.py) uses make_store directly. And the reference adapter in tests/package_with_entrypoint/__init__.py:114 (MemoryStore.open(read_only=False)) itself violates the ABC's "must honor context.read_only" — so the one example adapter authors will copy is wrong. Suggest the resolver enforce it (with_read_only(True) or raise) rather than trusting adapters. (Note ZipStore doesn't implement with_read_only, so a zip adapter returning a writable store under "r" would hit NotImplementedError via make_store_path too.)

4. mode="a" → "r" downgrade only happens for pipeline strings, and produces a late, unclear error (_common.py:488). Every other StoreLike gets StorePath.open's explicit Store is read-only but mode is 'a'. Create a writable store or use 'r' mode.; here make_store_path returns a read-only StorePath while open_group/open_array still hold mode="a", take the create path when the node doesn't exist, and die in Store._check_writable:

   zarr.open_group("rooty://x", mode="a")   # ValueError: store was opened in read-only mode and does not support writing

   test_mode_a_downgrades_to_read_only_open asserts this deliberately, so it's a design choice — but either "a on a read-only store means open-only" belongs in StorePath.open for all stores, or it shouldn't exist and adapters own the decision (which is what the PipelineContext.mode docstring already says).

5. fsspec :: chained URLs get captured by a same-named root adapter (_url_pipeline.py:117-118 via _root_scheme). parse_store_url("zip::file:///t/d.zip") yields scheme='zip', so as soon as any adapter named zip is registered (the planned builtin, or an entry point), zarr.open("zip::file:///t/d.zip") — which works today through FsspecStore.from_url — is rerouted to the adapter as a root segment with body=':file:///t/d.zip', preceding=(). Same for tar::, simplecache::, etc. Root-adapter dispatch should exclude scheme:: roots (or treat them as opaque fsspec URLs).

6. make_store tests the raw residual path (_common.py:359). An adapter returning path="/" makes make_store raise "resolves to a path inside a store" while make_store_path of the same URL normalizes it to "". AdapterResolution.path says root is "", so "/" is technically off-contract, but spec bodies are slash-prefixed (zip:/path/..., n5:/..., json:/) so it's worth normalize_path-ing defensively.

7. Entry-point names are never lowercased (registry.py:329,343, minor). register_url_adapter lowercases, but get_url_adapter compares the lowercased scheme against the raw entry_point.name and list_url_adapter_schemes unions raw names. An entry point named MyScheme is advertised but never loadable or routable — get_url_adapter("myscheme") raises no URL pipeline adapter is registered for scheme 'myscheme'. Registered schemes: ['MyScheme'].

8. Root-adapter URLs skip make_store's storage_options validation (_common.py:357). For rooty://x, storage_options are handed to the adapter and never checked as used, so make_store's documented TypeError contract no longer holds for those strings — fine if intended, but worth stating. (Direct make_store also skips the mode assert for root adapters, but that's a bare assert on main too and StorePath.open still validates for zarr.open, so not much lost.)

9. get_url_adapter's lazy-load loop is racy across threads and can permanently drop a scheme (registry.py:338-347). It iterates lazy_load_list while entry_point.load() (an import; releases the GIL) runs, then slice-assigns [:] = remaining. Two threads resolving different schemes interleave → one thread's rebuilt list omits entries the other shifted, and its final write clobbers. Reproduced with fake entry points [b, a(slow), c]: afterwards c is gone from both list_url_adapter_schemes() and routing for the rest of the process. Only reachable via the async API from multiple threads/loops (the sync API funnels through one IO thread), so low exposure — but a lock or a name-keyed dict of pending entry points would make it moot.

10. Wrapper adapters can't actually "consume adapter-specific keys" from storage_options (docstring at abc/url_pipeline.py:110-113 vs _url_pipeline.py:161). _resolver closes over the caller's original storage_options and resolve_preceding() takes no arguments, so there's no way to strip adapter keys before the root sees them; for a local/memory root the unused-options TypeError fires before the adapter runs (which test_storage_options_forwarded_to_root asserts), and for an fsspec root unknown keys reach the filesystem constructor. Native (preceding_url) adapters are unaffected. Either let resolve_preceding(storage_options=...) override, or drop the docstring claim.

Spec conformance at the root

11. Root sub-URL semantics are delegated wholesale to legacy make_store, which doesn't implement the spec's memory:/file: definitions (_url_pipeline.py:161). The spec says memory:, memory:/, memory:// and memory:a/memory:/a/memory://a are pairwise equivalent; with fsspec installed, make_store gives four different stores (memory: and memory:a become MemoryFileSystem keys under cwd, memory://a → /a), and different again without fsspec. file://localhost/tmp/x (≡ file:/tmp/x per spec) builds LocalStore("tmp/x") relative to cwd; file:relative/path (spec MUST NOT) is accepted; a query on file: is silently dropped. All pre-existing in make_store, but the PR now makes these the spec-mandated root of every pipeline, and test_spec_examples.py only exercises parse_pipeline — so conformance is asserted lexically while resolution disagrees. Notably memory://x|adapter: (the idiom throughout the PR's tests) is the only memory spelling that works as a root. The base case probably wants a small spec-aware root resolver for memory:/file:.

12. ?/# handling on bare-path roots (_url_pipeline.py:56-63, 90-96). URL lexical rules are applied to schemeless local paths, where those bytes are ordinary filename chars: parse_pipeline("/tmp/d?v=1|zip:") yields body='/tmp/d', query='v=1' while the base case passes raw to make_store, which strips ?v=1 and opens /tmp/d — no error, adapters inspecting preceding[0].body see something else. Simulated win32: \\?\C:\x|zip: splits into body='\\\\', query='C:\\x'. Either skip query splitting for schemeless roots or document that local roots may not contain ?/#. Related: _root_scheme uses urlparse (which strips leading whitespace and \t\r\n) while the body slice uses the raw string, so "\tfile:/x|zip:" yields scheme='file', body='\tfile:/x'; and roots with exotic authorities that the fallback regex accepts (vendor.x://[authority]/path, celebrated in test_exotic_authority_root_scheme) leak a raw ValueError: 'authority' does not appear to be an IPv4 or IPv6 address from parse_store_url if they ever reach the base case, instead of URLPipelineError.

Design / API surface

13. Root-adapter routing by entry-point name extends install-time scheme override to zarr's builtin routes. is_url_pipeline routes on entry-point names (unloaded included) with no builtin-scheme exclusion. fsspec already lets any installed package clobber s3/https/gcs via fsspec.specs, so that part isn't new — but file: and (without fsspec) memory: are, and the interception now happens in zarr before its own dispatch. Worth deciding whether builtin schemes should be excluded or an explicit opt-in required. Relatedly, collisions are silent: register_url_adapter overwrites unconditionally, get_url_adapter skips lazy loading when the key is already registered (so a builtin zip permanently shadows a third-party zip entry point that list_url_adapter_schemes keeps advertising), and two entry points with the same name → iteration order wins. A warning on collision (or override=) would help.

14. AdapterResolution has no channel for what the headline |zarr3: example must return. It's store + path only; a format adapter as last segment cannot express zarr_format, and make_store_path has nowhere to put it (the description says "StorePath gains a zarr_format attribute" but there's no such change in the diff). Adding a zarr_format: ZarrFormat | None = None slot later is source-compatible for constructors, but every wrapper adapter written against this release that re-wraps as AdapterResolution(store=..., path=r.path) will silently drop it. Since the entry-point contract is being frozen now, the slot (or an extras mapping, plus "use dataclasses.replace") probably belongs in this PR.

15. open_pipeline_segment runs on zarr's IO-loop thread, and the ABC doesn't say so. Under zarr.open("x|myscheme:") the adapter coroutine executes inside sync(), so any sync zarr call from an adapter (zarr.open, Group.open, anything using sync()) dies with SyncError: Calling sync() from within a running loop — reproduced with a trivial adapter — and blocking I/O in the adapter (zipfile central-directory read, sync fsspec.open) stalls every concurrent zarr op. Same constraint as Store._open, but this is a new third-party extension point whose most obvious implementation ("open the preceding thing with zarr, wrap it") is exactly the failing pattern; the docstring should say "must be non-blocking async, must not call the sync API; use resolve_preceding()".

16. The pipeline check is bolted on ahead of the existing dispatch rather than being a branch of it, and it's duplicated with diverging semantics (_common.py:357 vs :482 — only one has the "a" downgrade, only one has the residual-path check, only one enforces read-only), plus len(segments) == 1 and scheme not in list_url_adapter_schemes() is copy-pasted between resolve_pipeline and _resolve. zarr.open("memory://x") runs is_url_pipeline twice, parse_store_url three times, and builds the adapter-scheme set twice before any store exists (microseconds, but it's the duplication that matters). Routing on scheme once inside the existing parsed.scheme dispatch and letting make_store_path fall through to make_store would fix 3, 8, and 16 at once.

17. PipelineContext shape (abc/url_pipeline.py:118-122, nits): read_only is a stored field always equal to mode == "r" (a @property can't drift); _resolver is a required private field on a public frozen dataclass that adapter authors must supply in tests; frozen+dict is unhashable. Since the context already holds preceding/mode/storage_options, resolve_preceding could call the module-level _resolve(...) and drop _resolver and the per-segment closure.

18. Half-public surface. zarr.abc.url_pipeline and zarr.registry.*_url_adapter* are documented public, but parse_pipeline/resolve_pipeline live in zarr.storage._url_pipeline and aren't re-exported. Icechunk/tooling that wants to introspect a pipeline without opening a node has to import the underscore module. Either export from zarr.storage (and document) or state that zarr.open is the only supported entry.

Smaller things

• docs/api/zarr/abc/index.md doesn't list the new url_pipeline page (nav was updated). No user-guide entry describes the | syntax or how to register an adapter, though the checklist ticks it — maybe deferred to the adapter PRs?
• tests/test_url_pipeline/test_resolver.py: _store_of is unused; TracingStore.segment is set but never asserted. test_windows_drive_path_is_not_a_scheme only asserts segment.raw == input (true by construction) — could assert scheme under a sys.platform branch.
• The parser happy-path tests could be one parametrized test rather than ten functions.
• The description's "StorePath gains a zarr_format attribute (populated by format segments in a follow-up)" isn't in the diff (see 14).

[d-v-b]

a few more findings, this time from codex:

🤖 AI text below 🤖

A few additional findings on 6e6c2f5 that do not appear to overlap with the earlier review:

1. Rejected make_store() resolutions leak the opened store (src/zarr/storage/_common.py:357-364).

   When an adapter returns an open store with a nonempty residual path, make_store() raises URLPipelineError without closing that store. Reproduced with a close-tracking MemoryStore subclass:

   URLPipelineError
   closed=False open=True
   

   This could leak file handles, archive handles, or client sessions. The error branch should close result.store before raising, with a regression test using a tracking store.

2. The new property tests classify URLs outside the specification grammar as valid (tests/test_url_pipeline/test_parser.py:105-148).

   _BODY and _QUERY draw from nearly all printable ASCII, including raw spaces, backslashes, and malformed percent escapes. The scheme strategy also generates arbitrary unqualified nonstandard schemes. For example, both of these are accepted:

   file:/tmp/a b|zip:
   file:/tmp/%ZZ|custom:
   

   The URL-pipeline specification restricts the permitted URI characters and requires nonstandard schemes to use vendor.scheme naming. The public adapter example’s myscheme and the test entry point’s entrypoint-scheme also do not follow that convention.

   If parse_pipeline() is intentionally a permissive segment splitter rather than a grammar validator, that is defensible, but the property tests should not describe these generated values as valid/conforming. The validation boundary should also be documented explicitly.

3. The reference wrapper adapter discards the preceding residual path (tests/test_url_pipeline/test_resolver.py:39-50).

   It calls context.resolve_preceding(), wraps only preceding.store, then replaces preceding.path with segment.body:

   preceding = await context.resolve_preceding()
   store = TracingStore(preceding.store)
   return AdapterResolution(store=store, path=segment.body)

   Therefore a chain such as:

   root|wrap:a|wrap:b
   

   loses a for actual store operations. The API can represent this correctly, but the only wrapper example demonstrates the lossy pattern and there is no nested-wrapper resolution test. Before the zip: follow-up, it would be useful to document how wrapper adapters must consume preceding.path and add a test that proves residual paths survive nested wrappers.

For reference, the updated focused suite passes locally: 313 passed in 5.07s.