Add THREDDS catalog connector to the acquire stage#226
Closed
Hackshaven wants to merge 1 commit into
Closed
Conversation
Implements `zyra acquire thredds` to enumerate datasets from a THREDDS catalog.xml, map each dataset's urlPath to its HTTPServer (fileServer) download URL, and list, sync, or batch-fetch matches. Supports opt-in recursion into nested catalogRef entries (with a depth cap and cycle guard), plus pattern/date filtering and sync replacement options that mirror the existing FTP connector vocabulary. - New backend: connectors/backends/thredds.py (hermetic; supports a fetcher/catalog_xml injection so tests need no network) - CLI: acquire/import `thredds` subcommand - API: AcquireThreddsArgs/AcquireThreddsRun wired into POST /acquire - Tests: backend unit tests + CLI tests using offline catalog fixtures - Docs: ingest README THREDDS section; sample pipeline thredds_to_local.yaml - Regenerated capabilities manifest and OpenAPI sha256 snapshot Addresses NOAA-GSL/zyra#283 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Vfz8KKCKV7AGZ9RSPCiRoH Signed-off-by: Eric Hackathorn <erichackathorn@gmail.com>
|
🔁 Relay created/updated upstream PR: NOAA-GSL/zyra#285 |
Deploying zyra with
|
| Latest commit: |
cedeec3
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://323d92e8.zyra.pages.dev |
| Branch Preview URL: | https://claude-github-issue-283-oilp.zyra.pages.dev |
Contributor
There was a problem hiding this comment.
Pull request overview
Adds a first-class THREDDS (catalog.xml) connector to Zyra’s acquire/import stages, including a backend for catalog enumeration + fileServer URL mapping, CLI wiring, API schema/union updates, and hermetic tests/docs so users can list/sync/batch-fetch datasets from THREDDS catalogs.
Changes:
- Implement THREDDS backend (
parse_catalog, recursive enumeration, list, sync) and registerzyra acquire thredds/zyra import thredds. - Wire THREDDS into the API discriminated union and publish updated wizard capabilities + OpenAPI snapshot hash.
- Add offline unit/CLI tests, docs, and a sample pipeline.
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/snapshots/openapi_sha256.txt | Updates OpenAPI snapshot hash to reflect new API/CLI surface. |
| tests/connectors/test_thredds_backend.py | Adds hermetic unit coverage for catalog parsing, filtering, recursion, and sync behavior. |
| tests/cli/test_acquire_thredds.py | Adds CLI-level hermetic tests for list and sync flows. |
| src/zyra/wizard/zyra_capabilities/acquire.json | Adds generated capabilities entries for acquire thredds / import thredds. |
| src/zyra/wizard/zyra_capabilities.json | Updates aggregated generated capabilities to include THREDDS. |
| src/zyra/connectors/ingest/README.md | Documents new THREDDS connector usage patterns and options. |
| src/zyra/connectors/ingest/init.py | Registers new CLI subcommand and implements command handler logic. |
| src/zyra/connectors/backends/thredds.py | New backend implementing THREDDS catalog parsing/enumeration and sync logic. |
| src/zyra/api/schemas/domain_args.py | Adds AcquireThreddsArgs and resolves it for acquire/import; normalizes headers. |
| src/zyra/api/routers/domain_acquire.py | Extends acquire request union to include THREDDS tool. |
| src/zyra/api/models/domain_api.py | Adds AcquireThreddsRun request model. |
| samples/pipelines/thredds_to_local.yaml | Adds an end-to-end sample pipeline using THREDDS acquisition. |
Comment on lines
+212
to
+216
| try: | ||
| text = xml if xml is not None else fetch(url) | ||
| except Exception as exc: # pragma: no cover - network/error path | ||
| logger.warning("Failed to fetch THREDDS catalog %s: %s", url, exc) | ||
| return |
Comment on lines
+298
to
+318
| if options.skip_if_local_done and ftp_backend._has_done_marker(local_path): | ||
| return False | ||
| if not local_path.exists(): | ||
| return True | ||
| local_size = local_path.stat().st_size | ||
| if local_size == 0: | ||
| return True | ||
| if options.overwrite_existing or options.prefer_remote: | ||
| return True | ||
| if options.min_remote_size is not None or options.recheck_existing: | ||
| remote_size = http_backend.get_size( | ||
| download_url, headers=headers, timeout=timeout | ||
| ) | ||
| if remote_size is None: | ||
| return options.recheck_existing | ||
| threshold = ftp_backend._parse_min_size(options.min_remote_size, local_size) | ||
| if threshold is not None: | ||
| return remote_size >= threshold | ||
| if options.recheck_existing: | ||
| return remote_size != local_size | ||
| return False |
Comment on lines
+364
to
+379
| # Batch fetch of enumerated datasets | ||
| from pathlib import Path | ||
|
|
||
| urls = thredds_backend.list_files(ns.catalog_url, **common) | ||
| if ns.output_dir is None: | ||
| raise SystemExit( | ||
| "--output-dir is required when fetching from a THREDDS catalog" | ||
| ) | ||
| outdir = Path(ns.output_dir) | ||
| outdir.mkdir(parents=True, exist_ok=True) | ||
| for u in urls: | ||
| data = thredds_backend.fetch_bytes(u, headers=headers or None) | ||
| name = Path(u).name or "download.bin" | ||
| with (outdir / name).open("wb") as f: | ||
| f.write(data) | ||
| return 0 |
Comment on lines
+1228
to
+1231
| "--date-format", | ||
| dest="date_format", | ||
| help="Filename date format for filtering (e.g., YYYYMMDD)", | ||
| ) |
Comment on lines
+189
to
+211
| class AcquireThreddsArgs(BaseModel): | ||
| catalog_url: str | ||
| output: str | None = None | ||
| # Listing/sync/batch | ||
| list_mode: bool | None = Field(default=None, alias="list") | ||
| sync_dir: str | None = None | ||
| output_dir: str | None = None | ||
| # Enumeration | ||
| recursive: bool | None = None | ||
| max_depth: int | None = None | ||
| pattern: str | None = None | ||
| since: str | None = None | ||
| since_period: str | None = None | ||
| until: str | None = None | ||
| date_format: str | None = None | ||
| header: list[str] | None = None | ||
| # Sync mode options (subset meaningful over HTTP) | ||
| overwrite_existing: bool | None = None | ||
| recheck_existing: bool | None = None | ||
| min_remote_size: str | int | None = None | ||
| prefer_remote: bool | None = None | ||
| skip_if_local_done: bool | None = None | ||
|
|
Member
Author
|
Superseded by #227, which contains the same work (THREDDS connector + the review fixes) squashed into a single signed-off commit on a fresh branch. Closing this one to avoid duplication. Thanks for the review here — all five Copilot comments are addressed in #227. Generated by Claude Code |
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
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.
Summary
Adds a dedicated
zyra acquire threddsconnector that reads a THREDDS Data Servercatalog.xml, maps each dataset'surlPathto itsHTTPServer(fileServer) download URL, and lists, syncs, or batch-fetches matching datasets. This closes the workflow gap described in NOAA-GSL/zyra#283 — previouslyacquire httponly handled individual URLs, with no bulk catalog enumeration like S3/FTP offer.Matches the UX proposed in the issue:
What's included
src/zyra/connectors/backends/thredds.py—parse_catalog,enumerate_datasets,list_files,sync_directorysrc/zyra/connectors/ingest/__init__.py—threddssubcommand (registered foracquireandimport)AcquireThreddsArgs,AcquireThreddsRun, wired into thePOST /acquirediscriminated uniontests/connectors/test_thredds_backend.py,tests/cli/test_acquire_thredds.py(offline fixtures)README.mdTHREDDS section;samples/pipelines/thredds_to_local.yamlsha256snapshotDesign notes
acquire http) — THREDDS concepts (services,catalogRefrecursion,urlPath→fileServer mapping) don't map cleanly onto the HTTP anchor-scraper, and this matches thes3/ftpprecedent.--recursivefollows nestedcatalogRefentries up to--max-depth(default 3), with a visited-set cycle guard. Safer on large TDS servers.--pattern/--since/--until/--since-period/--date-formatand the sync replacement options (--overwrite-existing,--recheck-existing,--min-remote-size,--prefer-remote,--skip-if-local-done) reuse the existing FTP connector surface (SyncOptions).xml.etree.ElementTree+ the existingrequests(connectors extra). Tests are hermetic via afetcher/catalog_xmlinjection (mirrorsdiscovery/ogc.py).Validation
poetry run ruff format . && poetry run ruff check .— cleanzyra generate-manifestand./scripts/update_openapi_snapshot.shAddresses NOAA-GSL/zyra#283
🤖 Generated with Claude Code
Generated by Claude Code