apache / apache/iceberg-python

FsspecFileIO: `_adls` mutates shared properties, so a second storage account gets the first account's filesystem

未关闭 适合新手
#3,885 0 条评论 0 个 reaction 已指派 0 人 在 GitHub 查看
主要语言
Python
星标
1.1k
派生
581
平均合并
1 天 17 小时
30 天内合并 PR
78

描述

### Apache Iceberg version

main (development)

### Please describe the bug 🐞

`_adls` writes the account name it infers back into the properties dict it is handed, and the call site passes the FileIO's own `self.properties`. So the first ADLS location a `FsspecFileIO` touches pins `adls.account-name` for the life of that FileIO, and every later location is served a filesystem built for the first account.

In `pyiceberg/io/fsspec.py`:

```python
def _adls(properties: Properties, hostname: str | None = None) -> AbstractFileSystem:
...
# Fallback: extract account_name from URI hostname
if hostname and ADLS_ACCOUNT_NAME not in properties:
properties[ADLS_ACCOUNT_NAME] = hostname.split(".")[0]
```

and the caller at `fsspec.py:515`:

```python
if scheme in _ADLS_SCHEMES:
return _adls(self.properties, hostname)
```

The `lru_cache` on `(scheme, hostname)` just above it is not the problem. It does correctly build a second filesystem for a second hostname. But by then `ADLS_ACCOUNT_NAME` is already present in the shared properties, so the inference is skipped and the second filesystem gets the first account.

### Steps to reproduce

Two locations in two different storage accounts, starting from empty properties:

```python
from unittest.mock import patch
from pyiceberg.io.fsspec import FsspecFileIO

loc_a = "abfss://data@accountone.dfs.core.windows.net/wh/t/a.parquet"
loc_b = "abfss://data@accounttwo.dfs.core.windows.net/wh/t/b.parquet"

captured = []
class FakeFS:
def __init__(self, **kw): captured.append(kw.get("account_name"))

io = FsspecFileIO(properties={})
with patch("adlfs.AzureBlobFileSystem", FakeFS):
io.new_input(loc_a)
print(dict(io.properties))
io.new_input(loc_b)
print(dict(io.properties))

print(captured)
```

Output:

```
{'adls.account-name': 'accountone'}
{'adls.account-name': 'accountone'}
['accountone', 'accountone']
```

Expected `['accountone', 'accounttwo']`.

### What I expected

A FileIO given no `adls.account-name` should infer the account per location, not once. The inference is already per hostname at the cache layer, only the write into the shared dict breaks it.

The same mutation happens a few lines above in the SAS token loop, which sets `ADLS_ACCOUNT_NAME` from the token key, so that path has the same effect.

### Impact

A catalog whose tables span two storage accounts silently reads from the wrong account. Depending on whether a same named container exists there, this either fails with a confusing not found or resolves to the wrong data. It also means a FileIO's properties change as a side effect of reading, which is surprising for anything that inspects or reuses them.

### Suggested fix

Do not write into `properties`. Resolve the account into a local value inside `_adls` and pass that to `AzureBlobFileSystem`, leaving the caller's dict untouched. Happy to send a PR.

### Willingness to contribute

- [x] I can contribute a fix for this bug independently
- [ ] I would be willing to contribute a fix for this bug with guidance from the Iceberg community
- [ ] I cannot contribute a fix for this bug at this time

贡献指南

这个仓库没有索引到贡献指南

调研方向

从 pyiceberg/io/fsspec.py 中的 _adls 及其 515 行附近的调用方开始,然后使用模拟的 AzureBlobFileSystem 运行两个位置的复现。验证每个 hostname 都会生成自己的账户,并且在两次读取之后 io.properties 保持不变。

由索引模型根据 Issue 内容生成。

评估

技术栈
python
领域
backend, cloud
Issue 类型
缺陷
难度
2/5
预计耗时
1-3 小时
活跃度
活跃
描述清晰度
描述清楚
新手友好度
78/100

把新 issue 发到你的邮箱

精选适合新手参与的 GitHub issue 摘要。