fsspec / filesystem_spec

A specification that python filesystems should adhere to.
BSD 3-Clause "New" or "Revised" License
982 stars 345 forks source link

Upcoming aiohttp 4 breakages #819

Open sbesson opened 2 years ago

sbesson commented 2 years ago

See https://github.com/ome/ome-zarr-py/pull/127 for the original discussion.

The CI pre-releases workflow running on ome-zarr-py, a downstream consumer of fsspec identified an incompatibility between fsspec and the next major version of aiohttp where the loop keyword for the ClientSession constructor has been dropped:

WARNING  ome_zarr.io:io.py:190 exception on parsing: __init__() got an unexpected keyword argument 'loop' (stacktrace at DEBUG)
WARNING  ome_zarr.io:io.py:191 stacktrace:
Traceback (most recent call last):
  File "D:\a\ome-zarr-py\ome-zarr-py\ome_zarr\io.py", line 184, in parse_url
    loc = ZarrLocation(path, mode=mode, fmt=fmt)
  File "D:\a\ome-zarr-py\ome-zarr-py\ome_zarr\io.py", line 46, in __init__
    self.__store = loader.init_store(self.__path, mode)
  File "D:\a\ome-zarr-py\ome-zarr-py\ome_zarr\format.py", line 119, in init_store
    store = FSStore(
  File "D:\a\ome-zarr-py\ome-zarr-py\.tox\py39\lib\site-packages\zarr\storage.py", line 1132, in __init__
    self.map = fsspec.get_mapper(url, **storage_options)
  File "D:\a\ome-zarr-py\ome-zarr-py\.tox\py39\lib\site-packages\fsspec\mapping.py", line 228, in get_mapper
    fs, urlpath = url_to_fs(url, **kwargs)
  File "D:\a\ome-zarr-py\ome-zarr-py\.tox\py39\lib\site-packages\fsspec\core.py", line 399, in url_to_fs
    fs = cls(**options)
  File "D:\a\ome-zarr-py\ome-zarr-py\.tox\py39\lib\site-packages\fsspec\spec.py", line 76, in __call__
    obj = super().__call__(*args, **kwargs)
  File "D:\a\ome-zarr-py\ome-zarr-py\.tox\py39\lib\site-packages\fsspec\implementations\http.py", line 116, in __init__
    sync(self.loop, self.set_session)
  File "D:\a\ome-zarr-py\ome-zarr-py\.tox\py39\lib\site-packages\fsspec\asyn.py", line 71, in sync
    raise return_result
  File "D:\a\ome-zarr-py\ome-zarr-py\.tox\py39\lib\site-packages\fsspec\asyn.py", line 25, in _runner
    result[0] = await coro
  File "D:\a\ome-zarr-py\ome-zarr-py\.tox\py39\lib\site-packages\fsspec\implementations\http.py", line 133, in set_session
    self._session = await self.get_client(loop=self.loop, **self.client_kwargs)
  File "D:\a\ome-zarr-py\ome-zarr-py\.tox\py39\lib\site-packages\fsspec\implementations\http.py", line 29, in get_client
    return aiohttp.ClientSession(**kwargs)
TypeError: __init__() got an unexpected keyword argument 'loop'

aiohttp 4 is still in alpha mode so this won't impact consumers yet but this will likely be something to deal with when aiohttp 4.0.0 is released

Looking quickly at the how-to guides https://docs.aiohttp.org/en/stable/client_quickstart.html and https://docs.aiohttp.org/en/stable/client_advanced.html, it looks like it should be possible to anticipate this breaking change and refactor the way ClientSession are created with an event loop.

joshmoore commented 2 years ago

cc: @martindurant any ideas?

martindurant commented 2 years ago

I suppose the way to go will be to post a callback (loop.call_soon_threadsafe) which creates the client and attaches it to the file system instance.

martindurant commented 2 years ago

On second thoughts, get_client always runs as a coroutine, so I'm not sure its possible for it to run anywhere except the current event loop; so removing the argument may be all that we need.