huggingface / huggingface/datasets

xPath.glob passes double-wrapped storage options to url_to_fs

Open Beginner friendly
#8,543 2 comments 0 reactions 0 assignees View on GitHub
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

Open the contributing 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.