feat(data): integrate vosfs data commands - #217
Open
shinybrar wants to merge 58 commits into
Open
Conversation
feat(config): add named VOSpace services
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #217 +/- ##
==========================================
- Coverage 87.69% 87.48% -0.22%
==========================================
Files 59 62 +3
Lines 3698 4083 +385
Branches 440 502 +62
==========================================
+ Hits 3243 3572 +329
- Misses 332 365 +33
- Partials 123 146 +23
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. |
Contributor
Author
|
Implementation and automated review are complete. All required CI checks pass, the end-of-wave review is clean, and the xHigh five-lane mega review is clean after fix |
This was referenced Jul 22, 2026
shinybrar
marked this pull request as draft
July 22, 2026 22:10
shinybrar
changed the title
feat(storage): materialize authenticated vosfs sources
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
feat(discovery): discover primary VOSpace services
fix(storage): address wave 2 review findings
fix(server): harden VOSpace discovery and inspection
fix(ci): support pinned ty checks
test: support Python 3.10 TOML parsing
shinybrar
marked this pull request as ready for review
July 23, 2026 02:13
Rename the discovery seam for clarity and drop the last dataclasses in favour of pydantic models, per repo convention. - _discovery: load_registry_evidence -> discover, discover_storage_resource -> discover_storage (server_uri/server_url/server_name -> uri/url/name), prepare_enrichment_workers -> enrich, select_storage_resource -> select_storage; inline the single-use _registry_namespace helper. - RegistryEvidence and EnrichmentWorkers are frozen pydantic models. - Rename preferred_storage_leaf -> leaf across IdpInfo, IVOARegistrySearch, and RegistryEvidence. - auth.oidc: _refresh_parameters -> _refresh, _persist_refreshed_credential -> _persist. - Move _storage.py to storages.py, relocate the local filesystem adapter out of the CLI, and expose sources() so cli/data.py only mounts the Typer app. - Google-style docstrings on all touched functions. No behaviour change.
The CADC Science Platform Server shipped no default storage, and discovery named a discovered Service after its Server Name, so the arc Service was addressed as `canfar:` and vault was never configured at all. - Add both CADC VOSpace Services to the default Server, taken from the IVOA registry: arc (ws-uv.canfar.net/arc) and vault (cadc-west-01.canfar.net/vault). Both authenticate with the same cadc credentials, so `canfar data ls arc:/` and `vault:/` work out of the box. - Heal Servers saved by an older client on load: an entry keyed by Server Name is restored to its registry leaf and missing default Services are merged in. Scoped to the default Servers, so federated Servers keep their Server Name keys and deliberate Storage Names are left untouched. - Merge discovered Services by IVOA URI so rediscovery refreshes an existing entry in place instead of duplicating it under the Server Name. Verified against the live services with a CADC certificate.
fsspec-cli v0.6.0 hands parsing to Typer, a breaking upstream change. The consumed contract is unchanged: App(sources, capabilities=...), .typer_app, AsyncFilesystemSource, and the capability TypedDicts are all identical, and only the unused CommandExtension protocol was replaced. VOSpaceFileSystem keeps its token/certfile/asynchronous/storage_options constructor and aclose() through its v0.7.0 internal decomposition. Two upstream behaviours moved, so their tests were updated to match: - Disabled recursion.remove now omits the flag from `rm` entirely rather than registering and rejecting it, so `rm -R` reports `No such option: -R`. Verified the capability is still wired: `cp` registers -R/-r, `rm` does not. - `-h` is now a human-readable size flag for `ls` that requires a long listing, instead of an unsupported option. No production code changes.
Every claim below was reproduced against the CLI before documenting it. - Use the real Storage Names arc and vault instead of the stale `canfar:`, including in quick-start and cli-help. - Add a real cross-source example copying a 166K public test cutout from vault into an arc home directory. - Drop the dependency-pin section and the audited-commit block; point at the current tagged releases instead. - Simplify the source-mapping paragraph. - Correct `ls -h`: it is a supported flag that requires a long listing, not an unsupported option. `ll` is retained, as it is an upstream command. - Correct the recursive-removal diagnostic to `No such option: -R`.
`rm -R` is rejected by Typer, which renders its usage error through rich. The panel wraps to the terminal width and carries ANSI colour, so asserting the substring `No such option: -R` passed locally on a wide console and failed on every CI Python. Assert the width-independent behaviour instead (exit status 2, empty stdout) and pin the real invariant structurally: the recursion capability withholds -R/-r from `rm` while still registering them on `cp`.
Build every VOSpace filesystem with fsspec's listing cache enabled, so a command that walks a tree stops re-requesting the same directories. A repeated listing drops from a round trip to a dictionary lookup. Each command builds and closes its own filesystem, so a cached listing cannot outlive the command that produced it and no cross-command staleness is possible.
Explain the automatic listing cache and show how to cache file contents from Python, pointing the cache at /scratch on a CANFAR session since it is fast local disk that is cleared with the session. VOSpace serves ranged reads, so document MMapCache for caching a large cube one block at a time instead of downloading it whole; reading a FITS header transfers a single block. Both snippets were run against the live service before being documented.
Rename EnrichmentWorkers to Enrichment and cli.data._upstream_group to group. The group rename also renames the local variable it was assigned to inside _DataGroup._delegate, which would otherwise shadow the module-level function and raise UnboundLocalError. Making group public drops the private-access suppressions from its tests. No behaviour change.
The operand grammar used `Storage-Name` as its placeholder, which reads as the glossary noun rather than the slot it occupies, and does not cover the reserved `local` prefix. Call the placeholder `identifier` and state that it is either a Storage Name or `local`. Keeps the `Storage Name` glossary term from CONTEXT.md intact, since runtime errors use it verbatim.
The caching section claimed VOSpace serves ranged reads and recommended MMapCache for fetching a cube one block at a time. That is wrong and the advice was actively harmful. vosfs `_cat_file(path, start, end)` reads the whole object and slices it in memory (filesystem.py), because OpenCADC Cavern does not implement HTTP byte ranges (staging.py); the package sends no Range header anywhere. A block cache therefore issues one full download per block, so reading three headers out of one cube downloads that cube three times. Replace the recommendation with its opposite: cache whole files, use exactly one cache layer, and avoid block caches. Also record that chained cache layers do not compose - the inner layer is never filled and never serves. Adds the supporting research note under docs/agents/research/.
The previous correction blamed the missing byte ranges on OpenCADC Cavern and generalised that to every VOSpace Service. Verified live against both services, that is wrong: vault is backed by minoc and answers a ranged GET with 206, Accept-Ranges: bytes and Content-Range: bytes 0-2879/3542400, while arc returns the whole body and advertises no range support. The limitation that matters is in the client: vosfs sends no Range header, so a partial read downloads the whole object on either service, and block caches stay a pessimization today. State it that way, and record that teaching vosfs to send Range would unlock real byte-range reads on vault but not on arc. Corrects the same conflation in the research note.
The same concept was called three things: `data` in the CLI, "Storage Name" in configuration and errors, and `identifier` in the operand grammar. Settle on one vocabulary. - CONTEXT.md renames the term to Storage Identifier and records that the reserved `local` identifier addresses the machine the command runs on. - Rename canfar/storages.py to canfar/storage.py, matching the singular module convention and the noun the domain now uses. - Reword user-visible errors, docstrings, and docs to Storage Identifier, and make the operand placeholder `storage-identifier:/absolute/path` so it names the slot rather than borrowing the glossary noun. Deliberately unchanged: the `storage` key in the configuration file, which is already the settled word and whose format is an invariant; the `canfar data` command group, which names the action rather than the thing; and Server Name, which is a separate term. This is naming only. Verified the saved configuration still loads and both Storage Identifiers still resolve.
Upstream shinybrar/vosfs#335 teaches vosfs to send an HTTP Range header and use the response when the byte endpoint answers 206, closing the gap this project reported in shinybrar/vosfs#334. Verified against the live services: - vault (minoc) now serves real partial reads. A cat_file(path, 0, 2880) on a 3542400 byte cube sends `Range: bytes=0-2879` and receives `HTTP 206` with `Content-Range: bytes 0-2879/3542400`, transferring 2880 bytes. - MMapCache over a cat_file fetcher is genuinely partial: one ranged request, one block of four materialised, no whole-object fallback. On 0.7.0 the same operation downloaded the whole object per block. - arc (Cavern) serves no ranges and still falls back to a whole-object read that is sliced; slices remain correct, so nothing regresses there. - blockcache still refuses, because Range is honoured for byte reads and not through the file-object path: StagedReadFile has no blocksize attribute. Docs replace the "do not cache byte ranges" guidance with per-backend guidance, and the research note carries an update banner recording which of its 0.7.0 conclusions are superseded.
Document how to reach a VOSpace Service from Python: resolve the endpoint and credential from configuration, then use the vosfs fsspec filesystem. Covers filesystem operations, materialising a local path, the three cache tiers (whole file, byte range, RAM), the per-backend range table, integration with astropy, numpy, pandas, and dask, and the async form. The page states plainly that no canfar.storage module exists yet and points at the CLI guide for the equivalent surface, so it documents what ships rather than a planned API. Every example was executed against the live services before being written: astropy reports NAXIS=4 and shape (1, 96, 26, 16) for the test cutout, the APASS table converts to a 2373-row DataFrame, and dask's lazy mean matches numpy's to the last digit.
Add the public Python surface to canfar.storage so a Storage Identifier is
importable by name:
from canfar.storage import arc, vault, local
A module-level __getattr__ resolves any configured Storage Identifier to a
ready, authenticated synchronous filesystem, and __dir__ offers them for tab
completion. Alongside it: identifiers() lists what is addressable,
filesystem() builds one explicitly with an optional runtime credential, and
fetch() copies one object to local disk and returns its Path. The existing
sources() seam used by the data command is unchanged.
Credentials are materialized through fsspec's background event loop, so the
synchronous builder also works from inside a running loop.
__getattr__ checks membership before building rather than catching KeyError
from the build, so a failure to authenticate surfaces as itself instead of as
a misleading "no attribute" error.
Docs use the new imports, and the CLI guide no longer claims the release ships
no public storage API.
Standards axis findings from the two-axis review, plus the Storage Identifier
vocabulary sweep.
Documented-standard violations:
- Glossary: no `storage_name` identifier remains. `_resolve_storage(identifier)`,
`server_by_identifier`, `_validate_storage_identifiers`, and the "Storage
Service" docstring now use CONTEXT.md terms.
- Home: `canfar/_discovery.py` moves to `canfar/utils/registry.py`, the
location AGENTS.md and the architecture Module Map document, and no longer
sits beside the near-homonymous `canfar/utils/discover.py`.
- Models over dicts: `HTTPClient.build()` replaces the three hand-built
`client_kwargs` dicts, still omitting unset credentials so settings sources
keep filling them.
Smells:
- `RuntimeCredential` replaces `tuple[str | None, str | None]`, so the four
consumers no longer re-switch on which half is None.
- `_vospace` and `filesystem` share `_resolve` and `_build`; the synchronous
path reuses the asynchronous resolution through fsspec's loop.
- `Configuration.storage_identifiers()` owns the server/storage walk that
`sources()` and `identifiers()` both duplicated.
- `Workers` replaces the misleading `Enrichment`; `evidence`/`workers` no
longer collide with the public `server.discover`/`enrich`.
- `LOCAL` and `RESERVED_IDENTIFIERS` live once in `models/http.py`; the
validator now rejects every identifier that would shadow the module surface.
- `__all__` drives `__dir__`, so completion cannot drift.
Also drops `fetch()` from the public surface in favour of fsspec's `get_file`,
and gives `filesystem("local")` the `skip_instance_cache=True` every other
construction site uses.
Adds docs/agents/upstream-provenance.md recording that both distributions are
pinned to tagged Git refs on a personal fork, with the pin history.
This was referenced Jul 26, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Implements epic #150: named VOSpace services and the upstream
canfar dataintegration.Closes #150.
Delivered
Server.storageconfiguration,VOSpaceService, global Storage Name validation, and glossary definitions.vosfs.VOSpaceFileSystemsource factories with runtime token/certificate precedence, saved OIDC refresh, X.509 validation, fresh instances, and guaranteed cleanup.fsspec-climounted atcanfar data; every configured Storage Name is mapped on each invocation,local:is always included, and active Server Selection does not filter sources.Packaging and policy
dataextra):vosfs @ git+https://github.com/shinybrar/vosfs@v0.8.0fsspec-cli @ git+https://github.com/shinybrar/vosfs@fsspec-cli-v0.7.0#subdirectory=src/fsspec-clipip install canfar.mvremains unsupported; documented workflow iscp, verify, then separaterm.Review
Validation at final epic head
uv run pre-commit run --all-files: passed.uv run --no-sync ruff check . --no-cache: passed.uv run --no-sync ruff format --check .: passed.uv run ty check canfar: passed; the explicitly authorized pre-existing unused-ignore warning class is ignored, with no othertyrule weakened.uv run --no-sync pytest tests -m "not slow" --no-cov -q -o cache_dir=/tmp/canfar-pytest-cache: 811 passed.uv run --group docs mkdocs build: passed; the git-committers plugin logged its known GitHub API rate-limit warning without failing the build.uv build: sdist and wheel passed.canfarservice / valid saved Authentication Record and certificate were available.Upstream audit
vosfsv0.6.0 andfsspec-cli-v0.5.0tagged releases.9e5314db4706894d31d54d245392f43b9556cfbb.Update (2026-07-25)
Work added to this branch after the original description was written.
Upstream releases bumped
vosfsv0.6.0->v0.7.0,fsspec-cliv0.5.0->v0.6.0.fsspec-cliv0.6.0 hands parsing to Typer (breaking upstream). The consumed contract is unchanged:App(sources, capabilities=...),.typer_app,AsyncFilesystemSourceand the capabilityTypedDicts are identical; only the unusedCommandExtensionprotocol was replaced.VOSpaceFileSystemkeeps its constructor andaclose().recursion.removenow omits-R/-rfromrmentirely rather than rejecting them at runtime, and-hbecame a human-readable size flag forlsthat requires a long listing.arcandvaultboth configuredThe CADC Server shipped no default storage, and discovery named a discovered Service after its Server Name, so the ARC Service was addressed as
canfar:and Vault was never configured.arc(ws-uv.canfar.net/arc) andvault(cadc-west-01.canfar.net/vault).canfar data ls arc:/andvault:/work out of the box.Verified live against both services with a CADC certificate.
One vocabulary: Storage Identifier
The same concept was called three things —
data(CLI), "Storage Name" (config and errors),identifier(operand grammar). Now settled: Storage Identifier, withcanfar/storages.pyrenamed tocanfar/storage.pyand the operand placeholder writtenstorage-identifier:/absolute/path.Deliberately unchanged: the
storagekey in the configuration file (format invariant), thecanfar datacommand group, and Server Name.Caching
/scratchon a Session.MMapCachefor byte-range caching. That was wrong and is fixed.vosfssends noRangeheader, so a partial read downloads the whole object — a block cache issues one full download per block. The limit is in the client and is not uniform:vault(minoc) answers a rangedGETwith206andAccept-Ranges: bytes, whilearc(Cavern) does not. Filed upstream as Honour HTTP Range where the backend supports it (vault/minoc does; Cavern does not) shinybrar/vosfs#334.Housekeeping
mypyreplaced byty, scoped tocanfar/only ([tool.ty],astral-sh/ty-pre-commit). CI type-checks via the pre-commit action, so no separate CI step changed.EnrichmentWorkers->Enrichment,_upstream_group->group, discovery helpers shortened, remaining dataclasses converted to pydantic.docs/agents/research/2026-07-25-storage-python-api.md. No public Python API ships in this PR; it is scoped to a follow-up targeting 1.5.0.Gate
ruffclean,ty check canfarclean,pytest -m "not slow"819 passed at 89.00% coverage,mkdocs buildclean, all pre-commit hooks pass.Amended 2026-07-25
Landed after the epic was written, at the maintainer's request. The acceptance
criteria on #150, #151, #152, and #155 were amended to match.
vosfs@v0.8.0andfsspec-cli-v0.7.0; v0.8.0 addsserver-side HTTP
Range, honoured byvaultand not by Cavern behindarc.arcandvault, supersedingDiscover primary VOSpace services from registry records #152's "do not automatically add CADC Vault" and its Server-Name keying.
canfar.storage(
from canfar.storage import arc, vault, local, plusidentifiers(),filesystem(),sources()), superseding the "no public storage API"non-goal. Documented in
docs/client/data.md.origin/main; thestandards findings are addressed in
54a5fc65.Upstream pin provenance is recorded in
docs/agents/upstream-provenance.md,replacing the audited-commit note #155 asked for in the user docs.