From b77e1c2ae57f05f913c196160a7b69afabb1cf8d Mon Sep 17 00:00:00 2001 From: Nathan Zimmerman Date: Wed, 4 Dec 2024 12:17:44 -0600 Subject: [PATCH 01/10] Wrap sync fs for xarray.to_zarr --- src/zarr/storage/_fsspec.py | 11 +++++++++++ tests/test_store/test_fsspec.py | 17 +++++++++++++++++ 2 files changed, 28 insertions(+) diff --git a/src/zarr/storage/_fsspec.py b/src/zarr/storage/_fsspec.py index 99c8c778e7..752d237400 100644 --- a/src/zarr/storage/_fsspec.py +++ b/src/zarr/storage/_fsspec.py @@ -172,6 +172,17 @@ def from_url( opts = {"asynchronous": True, **opts} fs, path = url_to_fs(url, **opts) + if not fs.async_impl: + try: + from fsspec.implementations.asyn_wrapper import AsyncFileSystemWrapper + + fs = AsyncFileSystemWrapper(fs) + except ImportError as e: + raise ImportError( + f"The filesystem for URL '{url}' is synchronous, and the required " + "AsyncFileSystemWrapper is not available. Upgrade fsspec to version " + "2024.12.0 or later to enable this functionality." + ) from e # fsspec is not consistent about removing the scheme from the path, so check and strip it here # https://github.com/fsspec/filesystem_spec/issues/1722 diff --git a/tests/test_store/test_fsspec.py b/tests/test_store/test_fsspec.py index 2713a2969d..8d13d33dbc 100644 --- a/tests/test_store/test_fsspec.py +++ b/tests/test_store/test_fsspec.py @@ -6,6 +6,7 @@ import pytest from botocore.session import Session +from fsspec.implementations.asyn_wrapper import AsyncFileSystemWrapper import zarr.api.asynchronous from zarr.abc.store import OffsetByteRequest @@ -215,3 +216,19 @@ async def test_empty_nonexistent_path(self, store_kwargs) -> None: store_kwargs["path"] += "/abc" store = await self.store_cls.open(**store_kwargs) assert await store.is_empty("") + + +def test_wrap_sync_filesystem(): + """The local fs is not async so we should expect it to be wrapped automatically""" + store = FsspecStore.from_url("local://test/path") + + assert isinstance(store.fs, AsyncFileSystemWrapper) + assert store.fs.async_impl + + +def test_no_wrap_async_filesystem(): + """An async fs should not be wrapped automatically; fsspec's https filesystem is such an fs""" + store = FsspecStore.from_url("https://test/path") + + assert not isinstance(store.fs, AsyncFileSystemWrapper) + assert store.fs.async_impl From 616e9b21628e19f500aa77c07a7dcaf17f21ab15 Mon Sep 17 00:00:00 2001 From: Martin Durant Date: Wed, 22 Jan 2025 11:03:30 -0500 Subject: [PATCH 02/10] Move imports, skip tests on old fsspec and add changes note --- changes/2533.bigfix.rst | 1 + tests/test_store/test_fsspec.py | 14 +++++++++++++- 2 files changed, 14 insertions(+), 1 deletion(-) create mode 100644 changes/2533.bigfix.rst diff --git a/changes/2533.bigfix.rst b/changes/2533.bigfix.rst new file mode 100644 index 0000000000..dbcdf40e3c --- /dev/null +++ b/changes/2533.bigfix.rst @@ -0,0 +1 @@ +Wrap sync fsspec filesystems with AsyncFileSystemWrapper in xarray.to_zarr \ No newline at end of file diff --git a/tests/test_store/test_fsspec.py b/tests/test_store/test_fsspec.py index 8d13d33dbc..a560ca02e8 100644 --- a/tests/test_store/test_fsspec.py +++ b/tests/test_store/test_fsspec.py @@ -6,7 +6,7 @@ import pytest from botocore.session import Session -from fsspec.implementations.asyn_wrapper import AsyncFileSystemWrapper +from packaging.version import parse as parse_version import zarr.api.asynchronous from zarr.abc.store import OffsetByteRequest @@ -218,16 +218,28 @@ async def test_empty_nonexistent_path(self, store_kwargs) -> None: assert await store.is_empty("") +@pytest.mark.skipif( + parse_version(fsspec.__version__) < parse_version("2024.12.0"), + reason="No AsyncFileSystemWrapper", +) def test_wrap_sync_filesystem(): """The local fs is not async so we should expect it to be wrapped automatically""" + from fsspec.implementations.asyn_wrapper import AsyncFileSystemWrapper + store = FsspecStore.from_url("local://test/path") assert isinstance(store.fs, AsyncFileSystemWrapper) assert store.fs.async_impl +@pytest.mark.skipif( + parse_version(fsspec.__version__) < parse_version("2024.12.0"), + reason="No AsyncFileSystemWrapper", +) def test_no_wrap_async_filesystem(): """An async fs should not be wrapped automatically; fsspec's https filesystem is such an fs""" + from fsspec.implementations.asyn_wrapper import AsyncFileSystemWrapper + store = FsspecStore.from_url("https://test/path") assert not isinstance(store.fs, AsyncFileSystemWrapper) From c4e7a29add1f92dfc6eabcbf33664984c75ede31 Mon Sep 17 00:00:00 2001 From: Martin Durant Date: Wed, 22 Jan 2025 11:12:43 -0500 Subject: [PATCH 03/10] Perhaps fix labeler --- .github/workflows/needs_release_notes.yml | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/.github/workflows/needs_release_notes.yml b/.github/workflows/needs_release_notes.yml index 7a6c5462b4..10b719acc1 100644 --- a/.github/workflows/needs_release_notes.yml +++ b/.github/workflows/needs_release_notes.yml @@ -11,7 +11,12 @@ jobs: pull-requests: write runs-on: ubuntu-latest steps: - - uses: actions/labeler@8558fd74291d67161a8a78ce36a881fa63b766a9 # v5.0.0 - with: - repo-token: ${{ secrets.GITHUB_TOKEN }} - sync-labels: true + - uses: actions/checkout@v4 + # https://github.com/actions/labeler/issues/712#issuecomment-2454544085 + with: + clean: false + ref: ${{ github.event.pull_request.head.sha }} + - uses: actions/labeler@8558fd74291d67161a8a78ce36a881fa63b766a9 # v5.0.0 + with: + repo-token: ${{ secrets.GITHUB_TOKEN }} + sync-labels: true From 49cc874ee817fb950d84542f6d33ceb7205db1cd Mon Sep 17 00:00:00 2001 From: Martin Durant Date: Wed, 22 Jan 2025 11:15:10 -0500 Subject: [PATCH 04/10] revert labeler --- .github/workflows/needs_release_notes.yml | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/.github/workflows/needs_release_notes.yml b/.github/workflows/needs_release_notes.yml index 10b719acc1..ff5fef597b 100644 --- a/.github/workflows/needs_release_notes.yml +++ b/.github/workflows/needs_release_notes.yml @@ -11,12 +11,7 @@ jobs: pull-requests: write runs-on: ubuntu-latest steps: - - uses: actions/checkout@v4 - # https://github.com/actions/labeler/issues/712#issuecomment-2454544085 - with: - clean: false - ref: ${{ github.event.pull_request.head.sha }} - - uses: actions/labeler@8558fd74291d67161a8a78ce36a881fa63b766a9 # v5.0.0 + - uses: actions/labeler@v4 with: repo-token: ${{ secrets.GITHUB_TOKEN }} sync-labels: true From dde0b39fe64f5859059b813422733298f7a038cd Mon Sep 17 00:00:00 2001 From: Martin Durant Date: Wed, 22 Jan 2025 11:21:51 -0500 Subject: [PATCH 05/10] undo --- .github/workflows/needs_release_notes.yml | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/.github/workflows/needs_release_notes.yml b/.github/workflows/needs_release_notes.yml index ff5fef597b..7a6c5462b4 100644 --- a/.github/workflows/needs_release_notes.yml +++ b/.github/workflows/needs_release_notes.yml @@ -11,7 +11,7 @@ jobs: pull-requests: write runs-on: ubuntu-latest steps: - - uses: actions/labeler@v4 - with: - repo-token: ${{ secrets.GITHUB_TOKEN }} - sync-labels: true + - uses: actions/labeler@8558fd74291d67161a8a78ce36a881fa63b766a9 # v5.0.0 + with: + repo-token: ${{ secrets.GITHUB_TOKEN }} + sync-labels: true From 2a24c198261217399c7c1e86a4e486133c14efe2 Mon Sep 17 00:00:00 2001 From: Nathan Zimmerman Date: Wed, 22 Jan 2025 13:11:22 -0600 Subject: [PATCH 06/10] Trying labeler@v5 --- .github/workflows/needs_release_notes.yml | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/.github/workflows/needs_release_notes.yml b/.github/workflows/needs_release_notes.yml index 7a6c5462b4..6c987cb019 100644 --- a/.github/workflows/needs_release_notes.yml +++ b/.github/workflows/needs_release_notes.yml @@ -11,7 +11,13 @@ jobs: pull-requests: write runs-on: ubuntu-latest steps: - - uses: actions/labeler@8558fd74291d67161a8a78ce36a881fa63b766a9 # v5.0.0 - with: - repo-token: ${{ secrets.GITHUB_TOKEN }} - sync-labels: true + - name: Checkout repository + uses: actions/checkout@v4 + with: + ref: ${{ github.event.pull_request.head.sha }} + + - name: Run Pull Request Labeler + uses: actions/labeler@v5.0.0 + with: + repo-token: ${{ secrets.GITHUB_TOKEN }} + sync-labels: true \ No newline at end of file From 6b14a7bb38acc046b312edb4e0025e6ee5b0e5cb Mon Sep 17 00:00:00 2001 From: Nathan Zimmerman Date: Wed, 22 Jan 2025 13:14:10 -0600 Subject: [PATCH 07/10] Test labeler fix with checkout --- .github/labeler.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/labeler.yml b/.github/labeler.yml index 4dd680ee5a..59f905ffa6 100644 --- a/.github/labeler.yml +++ b/.github/labeler.yml @@ -1,4 +1,4 @@ -- needs release notes: +needs release notes: - all: - changed-files: - any-glob-to-any-file: 'changes/*.rst' From 391e550712afb14ac7d600b0e0f7257e8e27b2e4 Mon Sep 17 00:00:00 2001 From: Nathan Zimmerman Date: Wed, 22 Jan 2025 13:20:51 -0600 Subject: [PATCH 08/10] small yaml change (?) --- .github/workflows/needs_release_notes.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/needs_release_notes.yml b/.github/workflows/needs_release_notes.yml index 6c987cb019..4a36faaf4f 100644 --- a/.github/workflows/needs_release_notes.yml +++ b/.github/workflows/needs_release_notes.yml @@ -1,7 +1,7 @@ name: "Pull Request Labeler" on: - - pull_request_target + pull_request_target jobs: labeler: From 114122d73225bdaf66b1b0539adda16b6e0512b1 Mon Sep 17 00:00:00 2001 From: Nathan Zimmerman Date: Wed, 22 Jan 2025 13:25:01 -0600 Subject: [PATCH 09/10] Copying other, working, examples --- .github/workflows/needs_release_notes.yml | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/.github/workflows/needs_release_notes.yml b/.github/workflows/needs_release_notes.yml index 4a36faaf4f..2764a1c1ae 100644 --- a/.github/workflows/needs_release_notes.yml +++ b/.github/workflows/needs_release_notes.yml @@ -11,10 +11,7 @@ jobs: pull-requests: write runs-on: ubuntu-latest steps: - - name: Checkout repository - uses: actions/checkout@v4 - with: - ref: ${{ github.event.pull_request.head.sha }} + - uses: actions/checkout@v4 - name: Run Pull Request Labeler uses: actions/labeler@v5.0.0 From 7ce5580d2d3f85522b1e7ecb22eabc6e2171cd58 Mon Sep 17 00:00:00 2001 From: Nathan Zimmerman Date: Wed, 22 Jan 2025 13:31:25 -0600 Subject: [PATCH 10/10] Run on pull_request, not target --- .github/labeler.yml | 5 ++--- .github/workflows/needs_release_notes.yml | 4 +++- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/.github/labeler.yml b/.github/labeler.yml index 59f905ffa6..def7840fb9 100644 --- a/.github/labeler.yml +++ b/.github/labeler.yml @@ -1,4 +1,3 @@ needs release notes: - - all: - - changed-files: - - any-glob-to-any-file: 'changes/*.rst' +- changed-files: + - any-glob-to-any-file: 'changes/*.rst' diff --git a/.github/workflows/needs_release_notes.yml b/.github/workflows/needs_release_notes.yml index 2764a1c1ae..c1444aecfb 100644 --- a/.github/workflows/needs_release_notes.yml +++ b/.github/workflows/needs_release_notes.yml @@ -1,7 +1,7 @@ name: "Pull Request Labeler" on: - pull_request_target + pull_request jobs: labeler: @@ -12,6 +12,8 @@ jobs: runs-on: ubuntu-latest steps: - uses: actions/checkout@v4 + with: + ref: ${{ github.event.pull_request.head.sha }} - name: Run Pull Request Labeler uses: actions/labeler@v5.0.0