Enhance dataset version history and Zarr resolution alignment - #17
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThe client now resolves detailed dataset metadata from STAC, supports explicit multiresolution loading, and exposes Ceramic version and citation APIs. Legacy CID-only resolver APIs remain available. ChangesDataset versioning and multiresolution loading
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant dClimateClient
participant STACResolver
participant CeramicAPI
participant ZarrStore
dClimateClient->>STACResolver: resolve dataset details
STACResolver-->>dClimateClient: return CID, versions URL, and Zarr resolutions
dClimateClient->>ZarrStore: select and open resolution
dClimateClient->>CeramicAPI: list versions or retrieve commit
CeramicAPI-->>dClimateClient: return typed version data
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
dclimate_client_py/dclimate_client.py (1)
378-440: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winPopulate the new
DatasetMetadatafields.
resolvedcontains the STAC release metadata, but the metadata dictionary returned byload_datasetdoes not copy it. Callers therefore cannot accessversions_api, provenance, citation, stream, commit, label, citability, or retention data after loading a dataset.Add these fields to the STAC metadata dictionary and add a regression test.
Proposed fix
metadata = { ... + "versions_api": resolved.versions_api, + "provenance_api": resolved.provenance_api, + "citation_api": resolved.citation_api, + "stream_id": resolved.stream_id, + "commit_id": resolved.commit_id, + "version_label": resolved.version_label, + "is_citable": resolved.is_citable, + "retention_class": resolved.retention_class, }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@dclimate_client_py/dclimate_client.py` around lines 378 - 440, Update the metadata construction in the load_dataset flow after resolving the dataset, using the fields on ResolvedDatasetDetails to populate versions_api, provenance, citation, stream, commit, label, citability, and retention in the returned STAC metadata dictionary. Preserve existing metadata and add a regression test verifying these fields remain accessible after loading a dataset.dclimate_client_py/stac_server.py (1)
262-283: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winCorrect the resolver return-type migration notes.
Both functions now return
ResolvedDatasetDetails, but their migration notes still stateResolvedDataset. Update the notes to match the public API.
dclimate_client_py/stac_server.py#L262-L283: replaceResolvedDatasetin the migration note withResolvedDatasetDetails.dclimate_client_py/stac_catalog.py#L294-L322: replaceResolvedDatasetin the migration note withResolvedDatasetDetails.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@dclimate_client_py/stac_server.py` around lines 262 - 283, Update the migration notes for resolve_dataset_from_stac_server in dclimate_client_py/stac_server.py (lines 262-283) and its corresponding resolver in dclimate_client_py/stac_catalog.py (lines 294-322) to refer to ResolvedDatasetDetails instead of ResolvedDataset; no implementation changes are needed.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@dclimate_client_py/ceramic_api.py`:
- Around line 26-79: Run Ruff formatting on the ceramic_api.py file and commit
the resulting formatting-only changes, preserving the existing VerificationInfo
and DatasetVersion behavior.
---
Outside diff comments:
In `@dclimate_client_py/dclimate_client.py`:
- Around line 378-440: Update the metadata construction in the load_dataset flow
after resolving the dataset, using the fields on ResolvedDatasetDetails to
populate versions_api, provenance, citation, stream, commit, label, citability,
and retention in the returned STAC metadata dictionary. Preserve existing
metadata and add a regression test verifying these fields remain accessible
after loading a dataset.
In `@dclimate_client_py/stac_server.py`:
- Around line 262-283: Update the migration notes for
resolve_dataset_from_stac_server in dclimate_client_py/stac_server.py (lines
262-283) and its corresponding resolver in dclimate_client_py/stac_catalog.py
(lines 294-322) to refer to ResolvedDatasetDetails instead of ResolvedDataset;
no implementation changes are needed.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 49d85d0b-85d8-42f8-a7ac-831bf0e3ef67
📒 Files selected for processing (9)
README.mddclimate_client_py/ceramic_api.pydclimate_client_py/datasets.pydclimate_client_py/dclimate_client.pydclimate_client_py/stac_catalog.pydclimate_client_py/stac_server.pytests/test_ceramic_api.pytests/test_review_fu_httpx.pytests/test_stac_version_discovery.py
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #17 +/- ##
==========================================
+ Coverage 75.56% 77.59% +2.03%
==========================================
Files 16 17 +1
Lines 2050 2312 +262
Branches 374 407 +33
==========================================
+ Hits 1549 1794 +245
- Misses 350 358 +8
- Partials 151 160 +9 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
![da-code-reviewer[bot]](https://avatars.githubusercontent.com/u/78453614?s=60&v=4){kind=small}
| ipfs_cid=resolved.cid, | ||
| kubo_cas=self._kubo_cas, | ||
| zarr_group=zarr_group, | ||
| zarr_group=zarr_group or resolved.zarr_group, |
There was a problem hiding this comment.
MEDIUM
ResolvedDatasetDetails now carries the release-service fields, but the metadata dictionary built immediately below never copies them. Consequently load_dataset() omits versions_api, provenance/citation URLs, commit metadata, and retention data despite adding them to DatasetMetadata. Populate the STAC metadata dictionary from resolved.
| return await asyncio.to_thread( | ||
| resolve_dataset_from_stac, | ||
| catalog=self._stac_catalog, | ||
| collection=resolved_collection, |
There was a problem hiding this comment.
LOW
This fallback passes the collection directly to the catalog resolver, omitting the unique suffix-alias normalization used by load_dataset(). Thus list_dataset_versions(collection="era5", ...) fails when only ecmwf_era5 exists even though loading the same collection succeeds. Apply the same list_available_datasets()/unique-suffix resolution here.
| anchored_versions = filter_anchored_versions(listing.versions) | ||
| if not anchored_versions: | ||
| raise ValueError(f"No anchored versions found for dataset '{dataset}'") | ||
| anchored_versions.sort( |
There was a problem hiding this comment.
MEDIUM
The versions endpoint already returns releases in Ceramic event order, but this re-sorts them using snapshot-controlled timestamps and arbitrary string tie-breakers. Missing, equal, or skewed timestamps can therefore make an older commit appear latest. Preserve the service ordering and select the last anchored entry instead.
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (5)
dclimate_client_py/dclimate_client.py (5)
609-652: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winReuse unqualified-collection resolution on the catalog fallback.
When
organizationis omitted,_resolve_dataset_detailspassescollectiondirectly toresolve_dataset_from_stac. The existingload_datasetpath searches for a unique<organization>_<collection>catalog ID at Lines [418-429]. If the STAC server is unavailable and the catalog containsnoaa_aigfswhile the caller passesaigfs, the version APIs can fail even though dataset loading succeeds. Share the collection-normalization logic between both paths. Add a fallback test for an unqualified collection.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@dclimate_client_py/dclimate_client.py` around lines 609 - 652, Share the collection-normalization logic used by load_dataset with _resolve_dataset_details so the catalog fallback resolves an unqualified collection to its unique organization-prefixed catalog ID when organization is omitted. Preserve explicitly qualified collections and organization-provided behavior. Add a fallback test covering an unqualified collection such as aigfs resolving to noaa_aigfs when the STAC server is unavailable.
445-445: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winPreserve an explicitly supplied
zarr_group.When
zarr_group="", this expression selectsresolved.zarr_group. The loader normalizes""toNone, which represents the root group. Use anis Nonecheck.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@dclimate_client_py/dclimate_client.py` at line 445, Update the zarr_group selection to distinguish an explicitly supplied empty string from None: retain the caller’s zarr_group whenever it is not None, including "", and fall back to resolved.zarr_group only when it is None.
677-683: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winServer-side Request Forgery (CWE-918): Server-Side Request Forgery (SSRF)
Exploitability: Moderate
Validate
versions_apibefore making the request. STAC properties supply this URL without validation, and_request_jsonfollows redirects. Restrict the scheme and destination host, and revalidate redirect targets for both version endpoints atdclimate_client_py/dclimate_client.py:677-683and705-709.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@dclimate_client_py/dclimate_client.py` around lines 677 - 683, Update the version-fetching flows around list_versions_from_url and the corresponding endpoint near the second version request to validate details.versions_api before requesting it and revalidate every redirect target in _request_json. Allow only the approved URL scheme and destination host, rejecting unsafe initial or redirected targets for both version endpoints.
677-683: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winSecurity Misconfiguration (CWE-319): Cleartext Transmission of Sensitive Information
Reachability: External · Exploitability: Moderate
Require HTTPS for every version-request hop.
Validate
details.versions_apibefore both version requests. Reject non-HTTPS URLs and revalidate redirect targets, or disable automatic redirects._request_jsoncurrently performs no scheme validation and enables redirects, allowing HTTP requests or HTTPS-to-HTTP downgrades.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@dclimate_client_py/dclimate_client.py` around lines 677 - 683, Update the version-request flow around list_versions_from_url and _request_json to enforce HTTPS for the initial details.versions_api URL and every redirect target. Either validate each redirect before following it or disable automatic redirects and handle them with scheme validation; reject any HTTP or other non-HTTPS URL before making the request.
383-389: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winPropagate STAC release metadata through
load_dataset.DatasetMetadatadeclaresversions_api,provenance_api,citation_api,stream_id,commit_id,version_label,is_citable, andretention_class, but the STAC branch omits them. Copy these fields fromresolvedand add a regression test for a STAC-resolved release.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@dclimate_client_py/dclimate_client.py` around lines 383 - 389, Update the STAC resolution branch in load_dataset to populate DatasetMetadata with versions_api, provenance_api, citation_api, stream_id, commit_id, version_label, is_citable, and retention_class from resolved, matching the non-STAC path. Add a regression test covering a STAC-resolved release and asserting these metadata fields are preserved.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@dclimate_client_py/dclimate_client.py`:
- Around line 609-652: Share the collection-normalization logic used by
load_dataset with _resolve_dataset_details so the catalog fallback resolves an
unqualified collection to its unique organization-prefixed catalog ID when
organization is omitted. Preserve explicitly qualified collections and
organization-provided behavior. Add a fallback test covering an unqualified
collection such as aigfs resolving to noaa_aigfs when the STAC server is
unavailable.
- Line 445: Update the zarr_group selection to distinguish an explicitly
supplied empty string from None: retain the caller’s zarr_group whenever it is
not None, including "", and fall back to resolved.zarr_group only when it is
None.
- Around line 677-683: Update the version-fetching flows around
list_versions_from_url and the corresponding endpoint near the second version
request to validate details.versions_api before requesting it and revalidate
every redirect target in _request_json. Allow only the approved URL scheme and
destination host, rejecting unsafe initial or redirected targets for both
version endpoints.
- Around line 677-683: Update the version-request flow around
list_versions_from_url and _request_json to enforce HTTPS for the initial
details.versions_api URL and every redirect target. Either validate each
redirect before following it or disable automatic redirects and handle them with
scheme validation; reject any HTTP or other non-HTTPS URL before making the
request.
- Around line 383-389: Update the STAC resolution branch in load_dataset to
populate DatasetMetadata with versions_api, provenance_api, citation_api,
stream_id, commit_id, version_label, is_citable, and retention_class from
resolved, matching the non-STAC path. Add a regression test covering a
STAC-resolved release and asserting these metadata fields are preserved.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: ea86d1f9-444c-4e72-8e6e-cae1b4ba9dc4
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (4)
README.mddclimate_client_py/dclimate_client.pypyproject.tomltests/test_stac_version_discovery.py
🚧 Files skipped from review as they are similar to previous changes (2)
- tests/test_stac_version_discovery.py
- README.md
| ] # How the dataset was loaded | ||
| organization: Optional[str] | ||
| zarr_group: Optional[str] | ||
| versions_api: Optional[str] |
There was a problem hiding this comment.
MEDIUM
load_dataset() never copies the newly resolved version/provenance fields into its returned metadata dictionary. Even when STAC advertises these values, callers receive none of the fields declared here. Populate the STAC metadata from resolved, preserving values such as is_citable=False.
| return await asyncio.to_thread( | ||
| resolve_dataset_from_stac, | ||
| catalog=self._stac_catalog, | ||
| collection=resolved_collection, |
There was a problem hiding this comment.
MEDIUM
This fallback omits load_dataset()'s unique suffix resolution for unprefixed collections (for example, era5 → ecmwf_era5). Thus both version methods can fail during STAC-server fallback even though loading the same dataset succeeds. Apply the same collection normalization before calling the catalog resolver.
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
dclimate_client_py/dclimate_client.py (1)
459-464: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winPreserve an explicit root Zarr group.
Line 462 treats
""as absent and replaces it withresolved.zarr_group. An empty string can explicitly select the Zarr root group. Check forNoneinstead.Proposed fix
- zarr_group=zarr_group or resolved.zarr_group, + zarr_group=zarr_group if zarr_group is not None else resolved.zarr_group,🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@dclimate_client_py/dclimate_client.py` around lines 459 - 464, Update the zarr_group argument in the _load_dataset_from_ipfs_cid call to fall back to resolved.zarr_group only when zarr_group is None, preserving an explicit empty string as the root Zarr group.tests/test_stac_server_async.py (1)
35-35: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winDo not catch
BaseExceptionduring fixture cleanup.
BaseExceptioncatchesKeyboardInterrupt,SystemExit, and task cancellation. CatchExceptionso these control-flow exceptions propagate immediately. This also resolves Ruff BLE001.Proposed fix
- except BaseException as error: + except Exception as error:🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/test_stac_server_async.py` at line 35, Update the fixture cleanup handler around the existing `except BaseException as error` clause to catch `Exception` instead, preserving the current cleanup behavior while allowing KeyboardInterrupt, SystemExit, and cancellation to propagate.Source: Linters/SAST tools
dclimate_client_py/stac_server.py (1)
516-517: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winCorrect the migration note.
Line 516 states that this function returns
ResolvedDataset. Its signature and return value useResolvedDatasetDetails. Update the note so callers see the correct public contract.Proposed fix
- Changed in 0.6: returns ResolvedDataset; variant='' is treated as an + Changed in 0.6: returns ResolvedDatasetDetails; variant='' is treated as an🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@dclimate_client_py/stac_server.py` around lines 516 - 517, Correct the migration note for the affected function by replacing the incorrect ResolvedDataset type with ResolvedDatasetDetails, matching the function signature and returned value. Preserve the existing variant behavior description.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@dclimate_client_py/dclimate_client.py`:
- Around line 459-464: Update the zarr_group argument in the
_load_dataset_from_ipfs_cid call to fall back to resolved.zarr_group only when
zarr_group is None, preserving an explicit empty string as the root Zarr group.
In `@dclimate_client_py/stac_server.py`:
- Around line 516-517: Correct the migration note for the affected function by
replacing the incorrect ResolvedDataset type with ResolvedDatasetDetails,
matching the function signature and returned value. Preserve the existing
variant behavior description.
In `@tests/test_stac_server_async.py`:
- Line 35: Update the fixture cleanup handler around the existing `except
BaseException as error` clause to catch `Exception` instead, preserving the
current cleanup behavior while allowing KeyboardInterrupt, SystemExit, and
cancellation to propagate.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: aa501a6a-8622-480d-8c20-525ebb9865ab
📒 Files selected for processing (5)
README.mddclimate_client_py/dclimate_client.pydclimate_client_py/stac_server.pytests/test_stac_server_async.pytests/test_stac_version_discovery.py
🚧 Files skipped from review as they are similar to previous changes (2)
- tests/test_stac_version_discovery.py
- README.md
| return await asyncio.to_thread( | ||
| resolve_dataset_from_stac, | ||
| catalog=self._stac_catalog, | ||
| collection=resolved_collection, |
There was a problem hiding this comment.
MEDIUM
Preserve shorthand collection resolution on fallback. Unlike load_dataset, this helper never maps an unprefixed collection such as era5 to a unique catalog key such as ecmwf_era5. When the STAC API is unavailable, both new version methods therefore fail for collection names that normal dataset loading accepts. Apply the same suffix normalization before resolving the catalog.
| anchored_versions = filter_anchored_versions(listing.versions) | ||
| if not anchored_versions: | ||
| raise ValueError(f"No anchored versions found for dataset '{dataset}'") | ||
| anchored_versions.sort( |
There was a problem hiding this comment.
MEDIUM
Keep the service's authoritative version order. The versions endpoint returns commits in Ceramic event order, but this sort replaces that order with snapshot timestamps and an arbitrary commit-ID tie-breaker. Equal, corrected, or backfilled timestamps can make get_latest_anchored_version() return an older commit. Preserve the listing order and select the last anchored entry.
| dataset=dataset, | ||
| variant=variant, | ||
| server_url=self._stac_server_url, | ||
| client=self._get_stac_http_client(), |
There was a problem hiding this comment.
LOW
Close the STAC client for context-free version calls. The new methods do not require entering the async context—and the added README example constructs a bare client—but this creates a pooled AsyncClient whose only close path is __aexit__. Such calls leave the connection pool and sockets unclosed. Require context-managed use, provide an explicit aclose(), or use a temporary client here.
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
dclimate_client_py/stac_catalog.py (1)
295-301: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winCorrect the stale return-type migration note.
Line 305 states that this function returns
ResolvedDataset. The function now returnsResolvedDatasetDetails. This gives API consumers incorrect migration guidance.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@dclimate_client_py/stac_catalog.py` around lines 295 - 301, Update the migration note associated with resolve_dataset_from_stac to state that the function returns ResolvedDatasetDetails instead of ResolvedDataset, keeping the guidance consistent with the function’s declared return type.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@dclimate_client_py/stac_catalog.py`:
- Around line 295-301: Update the migration note associated with
resolve_dataset_from_stac to state that the function returns
ResolvedDatasetDetails instead of ResolvedDataset, keeping the guidance
consistent with the function’s declared return type.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: bf291694-7e88-4132-92e5-d51148f872c1
📒 Files selected for processing (11)
README.mddclimate_client_py/__init__.pydclimate_client_py/datasets.pydclimate_client_py/dclimate_client.pydclimate_client_py/dclimate_zarr_errors.pydclimate_client_py/ipfs_retrieval.pydclimate_client_py/stac_catalog.pydclimate_client_py/stac_server.pytests/test_ipfs_retrieval.pytests/test_stac_server_async.pytests/test_stac_version_discovery.py
🚧 Files skipped from review as they are similar to previous changes (1)
- dclimate_client_py/datasets.py
There was a problem hiding this comment.
Codex Automated Review
Found two actionable reliability/correctness issues.
Posted 1 inline comment(s).
Findings that could not be placed inline:
dclimate_client_py/dclimate_client.py:563[medium] Preserve the resolved version metadata.ResolvedDatasetDetailsnow carries the advertised version, provenance, citation, commit, and retention fields, butload_dataset()only copiesresolutioninto its returned metadata. Consequently every STAC load discards the newly addedDatasetMetadatafields. Populate them fromresolvedwhen constructingmetadata.
| try: | ||
| return await aresolve_dataset_from_stac_server( | ||
| collection=resolved_collection, | ||
| dataset=dataset, |
There was a problem hiding this comment.
MEDIUM
Provide cleanup for version-only clients. The new version methods can be called without entering the async context (as shown in the README), but this line creates and retains an httpx.AsyncClient that is only closed by __aexit__. Repeated short-lived clients therefore leak connection pools until garbage collection/process exit. Require context-manager use, expose an aclose() path, or use a temporary client for these calls.
| organization: typing.Optional[str] = None, | ||
| cid: typing.Optional[str] = None, | ||
| return_xarray: bool = False, | ||
| resolution: typing.Optional[str] = None, |
There was a problem hiding this comment.
MEDIUM
Preserve existing positional arguments. Inserting resolution before the existing zarr_group and shard_read_mode parameters reinterprets previously valid positional calls; for example, a positional "0" group is now treated as a human-readable resolution and rejected. Append the new parameter after existing parameters or provide compatibility handling.
| return await asyncio.to_thread( | ||
| resolve_dataset_from_stac, | ||
| catalog=self._stac_catalog, | ||
| collection=resolved_collection, |
There was a problem hiding this comment.
MEDIUM
Normalize shorthand collections on catalog fallback. Unlike load_dataset, this path passes an unprefixed collection such as era5 directly to the new-layout catalog resolver, which requires ecmwf_era5 unless organization is supplied. Both version methods therefore fail during STAC-server outages for shorthand names that normal loading accepts. Reuse the unique-suffix normalization before resolving.
| anchored_versions = filter_anchored_versions(listing.versions) | ||
| if not anchored_versions: | ||
| raise ValueError(f"No anchored versions found for dataset '{dataset}'") | ||
| anchored_versions.sort( |
There was a problem hiding this comment.
MEDIUM
Preserve Ceramic event order when selecting the latest version. The service returns versions in authoritative event-state order, while dataset timestamps may be corrected or backfilled and need not be monotonic. Re-sorting by timestamp and commit ID can therefore return an older commit as the latest anchored release; filter the response in place and select its final anchored entry.
| organization: typing.Optional[str] = None, | ||
| cid: typing.Optional[str] = None, | ||
| return_xarray: bool = False, | ||
| resolution: typing.Optional[str] = None, |
There was a problem hiding this comment.
MEDIUM
Preserve existing positional arguments. Inserting resolution before the existing zarr_group and shard_read_mode parameters reinterprets valid positional calls; a positional "0" group is now treated as a resolution and rejected. Make resolution keyword-only after the existing parameters or add compatibility handling.
| return await asyncio.to_thread( | ||
| resolve_dataset_from_stac, | ||
| catalog=self._stac_catalog, | ||
| collection=resolved_collection, |
There was a problem hiding this comment.
MEDIUM
Normalize shorthand collections on catalog fallback. Unlike load_dataset(), this path passes an unprefixed collection such as era5 directly to the new-layout catalog resolver. If the STAC lookup fails, both version methods reject shorthand names that normal loading resolves to ecmwf_era5. Reuse the unique-suffix normalization before resolving.
| anchored_versions = filter_anchored_versions(listing.versions) | ||
| if not anchored_versions: | ||
| raise ValueError(f"No anchored versions found for dataset '{dataset}'") | ||
| anchored_versions.sort( |
There was a problem hiding this comment.
MEDIUM
Preserve Ceramic event order when selecting the latest version. The versions endpoint returns history ordered by Ceramic event state, while timestamp is snapshot content and need not be monotonic. Re-sorting can therefore select an older commit as the latest anchored release. Filter in place and return the final anchored entry.
3 participants
This pull request introduces a robust, unified interface for dataset version history and provenance, integrating support for STAC-advertised version services and expanding the metadata returned when resolving datasets. The main changes include a new
ceramic_api.pymodule for version/provenance access, substantial enhancements to dataset resolution to return richer metadata, and new async methods in the client for version history. Existing resolution code is refactored to support these richer results, and documentation is updated to demonstrate the new features.Dataset version/provenance support:
dclimate_client_py/ceramic_api.pymodule that provides a Python interface for listing dataset versions, resolving exact releases, fetching citations, and building gateway URLs. This supports both Hydrogen and Tritium Ceramic APIs and allows STAC-aware applications to discover and use the correct version service automatically.DatasetMetadatatype and dataset resolution results to include fields such asversions_api,provenance_api,citation_api,stream_id,commit_id,version_label,is_citable, andretention_classfor richer metadata. [1] [2]Client and API changes:
list_dataset_versions()todClimateClient, which uses the STAC-advertised version service to list dataset releases, selecting the appropriate backend automatically.ResolvedDatasetDetailsobject with extended metadata, and updated all call sites accordingly. [1] [2] [3] [4] [5] [6] [7] [8]Documentation:
README.mdwith a new section and example for using the version history interface, highlighting how STAC-aware clients can now discover and use the correct version service without hardcoding URLs.Minor code cleanup:
hydrogen_endpointvariable fromdatasets.py.These changes make it much easier for downstream users and applications to access dataset version history, provenance, and citation data in a backend-agnostic and future-proof way.
Summary by CodeRabbit
New Features
Bug Fixes
Documentation