Skip to content

fix(data_catalog): reading from private bucket - #1536

Merged
JoerivanEngelen merged 23 commits into
mainfrom
fix/permission_private_bucket
Sep 29, 2026
Merged

JoerivanEngelen merged 23 commits into
mainfrom
fix/permission_private_bucket

Conversation

@LuukBlom

@LuukBlom LuukBlom commented Sep 3, 2026 •

Copy link
Copy Markdown
Collaborator

This PR adds filesystem-aware reading across HydroMT readers and improves support for private cloud storage.

Note that the vast majority of lines changed are in pixi.lock, added testing, refactoring the readers, and updating callers of the readers.

Issue addressed

Fixes #1530
Fixes #1542

Explanation

Took Dirks changes as a starting point.

  • Sonarcloud raised an issue that all Xarray-based drivers had a bunch of duplication, so I extracted the duplicated reading logic into a single function: hydromt.data_catalog.drivers.xarray_options._read_xarray
  • Add handling for private bucket access in readers.py and xarray/zarr drivers.
  • Add aws profile configuration to tests.yml
  • Add gh secrets for the Access/Secret keys
  • Add boto3 and dask[distributed] to deps
  • Add tests/data_catalog/test_private_bucket.py that reads from the (now still) private hydromt-data minio bucket
  • Added open_zarrs & open_mfdataset to hydromt.readers

Edit:
After deciding to add AbstractFilesystem to the args for all readers:

  • Added a shared filesystem I/O layer hydromt._fsio.py for opening handles, reading bytes, creating Zarr mappers, closing lazy resources, and normalizing permission errors.
  • Updated hydromt.readers.py: raster, vector, tabular, NetCDF, Zarr, and multi-file readers to accept fsspec filesystems.
  • Resolved driver storage_options once at the filesystem boundary.
  • Added clear errors for unsupported remote formats and local-only readers.
  • Fixed lazy remote raster and xarray resources so handles remain open until the returned object is closed.
  • Added optional I/O dependencies for AWS and HDF5 support.
  • Added tests using in-memory filesystem and permission-handling tests.
  • Expanded cloud-storage documentation, including Azure Blob Storage guidance.

Most relevant files to review:

  • hydromt/_fsio.py: Shared fsspec utilities for remote readeing
  • hydromt/readers.py: Reader implementations updated to consume filesystems
  • tests/test_readers_filesystem.py: In-memory filesystem tests covering remote reads, lazy loading
  • tests/data_catalog/test_private_bucket.py: Private S3 integration and permission-handling tests

@DirkEilander DirkEilander left a comment •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks for this PR @LuukBlom!

I comfired it is working for zarr and tif, but not yet for nc files. See code snippet below for a test.

It would be good to also add some examples to the docs as there are some specificities that are good for users to know. Like nc only works with the "h5netcdf" engine, not with "netcdf4".

I also tested for vector files where I think another issue (#1542) related to MinIO specific (or different endpoint) seems to be the issue. I made a seperate issue for that one as I can imagine it requires a new PR.

import s3fs
import xarray as xr
from hydromt import DataCatalog

s3_path = "s3://hydromt-data/test/chirps-v2.0.1981.days_p05.nc"

## test with Hydromt DataCatalog
dc = DataCatalog().from_dict(
    {
        "test": {
            "data_type": "RasterDataset",
            "uri": s3_path,
            "driver": {
                "name": "raster_xarray",
                "filesystem": {
                    "protocol": "s3",
                    "anon": False,
                    "profile": "hydromt-data",
                },
                # NOTE: requires h5netcdf engine
                "options": {"engine": "h5netcdf"},
            },
        },
    }
)
try:
    ds = dc.get_rasterdataset("test")
    print(ds)
except Exception as e:
    print(e)
    # > PermissionError: Forbidden


## test with s3fs and xarray (this works)
fs = s3fs.S3FileSystem(anon=False, profile="hydromt-data")
with fs.open(s3_path, mode="rb") as s3_file:
    ds = xr.open_dataset(s3_file, engine="h5netcdf")
    print(ds)

@ClaireDons ClaireDons left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey, I only managed to partly review it for now and mainly looked at the docs, which look mostly fine. Although in the future it might be nice to have a full example in the working with models section that gets data from an S3 bucket. @JoerivanEngelen will review the rest!

Comment thread docs/user_guide/data_catalog/data_azure_blob_storage.rst Outdated

@JoerivanEngelen JoerivanEngelen left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good. I like the generalization of the readers.

I have some small comments though.

Comment thread hydromt/data_catalog/drivers/dataset/dataset_driver.py
Comment thread hydromt/data_catalog/drivers/dataset/xarray_driver.py Outdated
Comment thread hydromt/data_catalog/drivers/geodataframe/pyogrio_driver.py
Comment thread hydromt/data_catalog/drivers/raster/rasterio_driver.py Outdated
Comment thread hydromt/data_catalog/drivers/xarray_options.py Outdated
Comment thread hydromt/_fsio.py Outdated
Comment thread hydromt/readers.py
Comment thread hydromt/readers.py Outdated
Comment thread hydromt/readers.py
Comment thread hydromt/readers.py Outdated

@DirkEilander DirkEilander left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've tested that vector files work. Found one small potential issue in the docs.
Will try to also test nc later today.

Comment thread docs/user_guide/data_catalog/data_cloud_storage.rst Outdated
@DirkEilander

Copy link
Copy Markdown
Contributor

I've tested that vector files work. Found one small potential issue in the docs. Will try to also test nc later today.

NC and tif files also work in my tests. This PR can be merged I think. Thanks @LuukBlom

@JoerivanEngelen
JoerivanEngelen merged commit 53aa0af into main Sep 29, 2026
34 checks passed
@JoerivanEngelen
JoerivanEngelen deleted the fix/permission_private_bucket branch September 29, 2026 13:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

reading vector data from minio bucket does not work HydroMT DataCatalog does not read from private cloud buckets

4 participants