huggingface / huggingface/datasets
xPath.glob passes double-wrapped storage options to url_to_fs
- Dominant language
- Python
- Stars
- 22k
- Forks
- 3.4k
- Avg merge
- 5d 7h
- Merged PRs (30d)
- 17
Description
### Describe the bug
`xPath.glob` adds a second protocol key to the storage options before passing them to `url_to_fs`, so fsspec receives `https={'https': {...}}` instead of `https={...}`.
`_prepare_path_and_storage_options` already returns them protocol keyed, and the docstring of `_prepare_single_hop_path_and_storage_options` states that contract: "Storage options are formatted in the form {protocol: storage_options_for_protocol}". The sibling helpers `xglob`, `xwalk` and `xlistdir` pass the returned value straight through, and only `xPath.glob` re-keys it (`file_utils.py:1157`). `xPath.rglob` delegates to `glob`, so it is affected too.
The case this breaks is the one the comment two lines above the call describes, "globbing inside a zip in a private repo requires authentication": the options meant for the remote hop end up nested one level too deep, so fsspec does not apply them. I did not run this against a real private archive, so the effect on authentication is read from the `url_to_fs` call rather than observed. The nesting itself is measured below.
### Steps to reproduce the bug
```python
from unittest.mock import MagicMock, patch
from datasets.download.download_config import DownloadConfig
from datasets.utils.file_utils import xglob, xPath
download_config = DownloadConfig(storage_options={"https": {"block_size": "omit"}})
with patch("datasets.utils.file_utils.url_to_fs") as m:
m.return_value = (MagicMock(glob=lambda _: [], protocol="zip"), "")
list(xPath("zip://::https://domain.org/data.zip").glob("*", download_config=download_config))
print("xPath.glob:", m.call_args.kwargs)
with patch("datasets.utils.file_utils.url_to_fs") as m:
m.return_value = (MagicMock(glob=lambda _: [], protocol="zip"), "")
xglob("zip://*::https://domain.org/data.zip", download_config=download_config)
print("xglob: ", m.call_args.kwargs)
```
On main this prints `xPath.glob: {'https': {'https': {'block_size': 'omit', 'client_kwargs': {'trust_env': True}}}}` while the sibling prints `xglob: {'https': {'block_size': 'omit', 'client_kwargs': {'trust_env': True}}}`.
### Expected behavior
Both should pass `{'https': {'block_size': 'omit', 'client_kwargs': {'trust_env': True}}}` to `url_to_fs`.
### Environment info
- `datasets` 5.0.2.dev0, `main` at `bff13713a1e63a57a4793273df7665ce5f17a413`
- Python 3.11, Linux
Contributor guide
Research direction
Start in datasets/utils/file_utils.py around line 1157, then compare xPath.glob with xglob, xwalk, and xlistdir and follow xPath.rglob into glob. Run the provided mocked url_to_fs reproduction; done means xPath.glob and xPath.rglob pass the protocol-keyed storage options without an extra nesting level.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- python
- Domain
- backend
- Issue type
- Bug
- Difficulty
- 2/5
- Estimated time
- 1-3 hours
- Activity status
- Active
- Clarity
- Clearly specified
- Newbie friendliness
- 88/100