Allow to open netcdf file from remote HTTPS server #1083
Conversation
| """ | ||
| fs_path = "download/10.5880.GFZ.1.4.2023.006-VEnuo/GAMIv2-0_2010-2020_100m.nc" | ||
| store = new_data_store("https", root="datapub.gfz-potsdam.de") | ||
| ds = store.open_data(fs_path, chunks={}) |
There was a problem hiding this comment.
This loads the data from remote source which is not wanted in a unittest. I would suggest using pytest-recording, like in xcube-stac. This would mean that we add a new dependency pytest-recording to the test suite. Would that be okay? Or do you have a better suggestion?
There was a problem hiding this comment.
Not ok. Unit tests should not depend on external resources. Replace by memory filesystem in that you first write the un-chunked netcdf file.
There was a problem hiding this comment.
Oops, I misunderstood the intent of this PR. It is just about HTTP access. Yes, fine, use pytest-recording.
There was a problem hiding this comment.
@forman pytest-recording does not recognize the request when opening the data with xr.open_dataset. Therefore no cassette is written and it just uses the actual https request. I don't think we can use pytest-recording in this case. Probably we still need to build a mock, right?
There was a problem hiding this comment.
I mocked the result of xarray.open_dataset now.
| """ | ||
| fs_path = "download/10.5880.GFZ.1.4.2023.006-VEnuo/GAMIv2-0_2010-2020_100m.nc" | ||
| store = new_data_store("https", root="datapub.gfz-potsdam.de") | ||
| ds = store.open_data(fs_path, chunks={}) |
There was a problem hiding this comment.
Not ok. Unit tests should not depend on external resources. Replace by memory filesystem in that you first write the un-chunked netcdf file.
| """ | ||
| fs_path = "download/10.5880.GFZ.1.4.2023.006-VEnuo/GAMIv2-0_2010-2020_100m.nc" | ||
| store = new_data_store("https", root="datapub.gfz-potsdam.de") | ||
| ds = store.open_data(fs_path, chunks={}) |
There was a problem hiding this comment.
Oops, I misunderstood the intent of this PR. It is just about HTTP access. Yes, fine, use pytest-recording.
CHANGES.md
Outdated
| * The `DatasetNetcdfFsDataAccessor` class has been adjusted, so that NetCDF files can | ||
| be now opened from a remote HTTPS server using the `"https"` data store. |
There was a problem hiding this comment.
Please take the perspective of a user. The user facing API in this case is the data store framework. How is it improved?
Co-authored-by: Norman Fomferra <norman.fomferra@brockmann-consult.de>
konstntokas
deleted the
konstntokas-xxx-allow_remote_https_lazy_access_netcdf
branch
This PR adjusts the
DatasetNetcdfFsDataAccessorclass, so that NetCDF files can be now opened from a remote HTTPS server. It adjusts the file uri tohttp(s)://*#mode=bytes, which allows to simply usexarray.open_datasetto open the data remotely. This also supports lazy when settingchunks={}or to a specific tile size, as used before.Checklist:
Add docstrings and API docs for any new/modified user-facing classes and functionsNew/modified features documented indocs/source/*CHANGES.md