feat: Add Zarr v2 metadata access and basic auth support for OIDC-protected assets#2112
Open
feat: Add Zarr v2 metadata access and basic auth support for OIDC-protected assets#2112
Conversation
cauriol
changed the title
Contributor
Collaborator
|
Code Coverage (Ubuntu)DetailsDiff against developResults for commit: ca62273 Minimum allowed coverage is ♻️ This comment has been updated with latest results |
Collaborator
|
Code Coverage (Windows)DetailsDiff against developResults for commit: ca62273 Minimum allowed coverage is ♻️ This comment has been updated with latest results |
cauriol
changed the title
Collaborator
|
@cauriol, instead of introducing a new method @sbrunato can we get your opinion here ? |
Collaborator
@alambare I agree. |
alambare
requested changes
Apr 2, 2026
Comment on lines
+589
to
+595
| def request_asset( | ||
| self, | ||
| url: str, | ||
| ) -> requests.Response: | ||
| """Perform a GET request to the given URL using product's authentication headers.""" | ||
| headers = self.get_storage_options().get("headers", {}) | ||
| return requests.get(url, headers=headers, stream=True) |
Collaborator
There was a problem hiding this comment.
we should use the stream download method from EODAG download plugins instead of creating a new method.
| ) | ||
|
|
||
| if ".zmetadata" in mapper: | ||
| meta = json.loads(mapper[".zmetadata"]) |
Collaborator
There was a problem hiding this comment.
Use orjson instead of json.
Comment on lines
+614
to
+618
| # TODO: Support Zarr v3 when test data becomes available. | ||
| # Zarr v2 uses `.zmetadata`, while Zarr v3 exposes `zarr.json`. | ||
| # The implementation should be straightforward once we can validate it | ||
| # against real examples. | ||
| raise ValueError(f"No Zarr metadata file found at {base_url}") |
Collaborator
There was a problem hiding this comment.
With Destination Earth, we do have a zarr v3 store: DanubeHis data. I don't understand what is blocking here.
| headers = self.get_storage_options().get("headers", {}) | ||
| return requests.get(url, headers=headers, stream=True) | ||
|
|
||
| def list_zarr_files_from_metadata( |
Collaborator
There was a problem hiding this comment.
In zarr, we have keys and not files. I suppose you can rename the method to something like list_zarr_keys ?
| try: | ||
| url = self.assets[asset_key]["href"] if asset_key else self.location | ||
| except KeyError as e: | ||
| raise DatasetCreationError(f"{asset_key} not found in {self} assets") from e |
Collaborator
There was a problem hiding this comment.
You are not creating a xarray here. The exception class is not accurate.
| "zipstream-ng", | ||
| "fsspec", | ||
| "aiohttp", | ||
| "requests" |
Collaborator
There was a problem hiding this comment.
requests is already a dependency in EODAG.
Comment on lines
+605
to
+608
| mapper = fsspec.get_mapper( | ||
| base_url, | ||
| client_kwargs={"headers": headers, "trust_env": False}, | ||
| ) |
Collaborator
There was a problem hiding this comment.
I am not sure about introducing fsspec in EODAG library.
The library is a wrapper above multiple backends. To use it, you need to bring backends to support all the filesystems: s3fs for S3, gcsfs for google cloud storage and adlfs for azure blob storage.
Most of the times, users will store zarr files on a S3 compatible store but some may store them on google or azure stores.
Maybe we don't want to support google / azure stores ? or in an optional dependencies branch ?
My opinion:
- we do not support google and azure yet. Maybe once if we replace
boto3byobstore. - we do not bring
fsspecin EODAG but instead callboto3to get the list of zarr keys. If a dedicated function is needed to get the list of keys with boto3, it should live ineodag/utils/s3.py.
@sbrunato what do you think?
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
5 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.
This PR adds support for accessing Zarr v2 assets from EOProduct and extends OIDC authentication handling to support Basic authorization for protected asset access.
Changes:
-add helper methods in EOProduct to:
- extract auth headers from auth objects
- request protected assets with forwarded authentication headers
- list files from Zarr v2 metadata using .zmetadata
Notes
this PR currently targets Zarr v2 only through .zmetadata
Zarr v3 support can be added later if needed