_prepare_credentials: connector fix for fsspec>=2022.12

[dtrifiro]

• move connector configuration to get_client
• fixes deprecation of fsspec_loop in fsspec #24

[dtrifiro]

The above seems to work, but when running in debug mode (PYTHONASYNCIODEBUG=1), we get a fatal error due to weakref:

import os

from dvc_http import HTTPFileSystem

os.environ["PYTHONASYNCIODEBUG"] = "1"


fs = HTTPFileSystem()
fs.get(
    "https://wtfismyip.com/json", os.path.join(os.getcwd(), "wtfismyip.json")
)

Traceback (most recent call last):
  File "/Users/dtrifiro/work/dvc-plugins/dvc-http/connector_testing.py", line 8, in <module>
    fs = HTTPFileSystem()
  File "/Users/dtrifiro/work/dvc-plugins/dvc-http/.venv/lib/python3.10/site-packages/dvc_objects/fs/base.py", line 78, in __init__
    self.fs_args.update(self._prepare_credentials(**kwargs))
  File "/Users/dtrifiro/work/dvc-plugins/dvc-http/dvc_http/__init__.py", line 83, in _prepare_credentials
    client_kwargs["connector"] = aiohttp.TCPConnector(
  File "/Users/dtrifiro/work/dvc-plugins/dvc-http/.venv/lib/python3.10/site-packages/aiohttp/connector.py", line 771, in __init__
    super().__init__(
  File "/Users/dtrifiro/work/dvc-plugins/dvc-http/.venv/lib/python3.10/site-packages/aiohttp/connector.py", line 267, in __init__
    self._cleanup_closed()
  File "/Users/dtrifiro/work/dvc-plugins/dvc-http/.venv/lib/python3.10/site-packages/aiohttp/connector.py", line 405, in _cleanup_closed
    self._cleanup_closed_handle = helpers.weakref_handle(
  File "/Users/dtrifiro/work/dvc-plugins/dvc-http/.venv/lib/python3.10/site-packages/aiohttp/helpers.py", line 611, in weakref_handle
    return loop.call_at(when, _weakref_handle, (weakref.ref(ob), name))
  File "/usr/lib/python3.10/asyncio/base_events.py", line 734, in call_at
    self._check_thread()
  File "/usr/lib/python3.10/asyncio/base_events.py", line 792, in _check_thread
    raise RuntimeError(
RuntimeError: Non-thread-safe operation invoked on an event loop other than the current one

I'll investigate a bit more before merging.

[skshetry]

Looks like the loop is closed before garbage collection.

[skshetry]

Okay that above error is a bug in fsspec's implementation or our config handling. aiohttp only tries to close connector if connector_owner is set to True, but we disable it and leave it to fsspec to close. close_session does not try to close connector if loop is running and session.close() is successful. So it never gets closed when weakref.finalize runs.

I have no idea why we set connector_owner to False.

[dtrifiro]

@skshetry simplified the implementation a bit. Also took the occasion to test get rid some of the changes from iterative/dvc#7414:

• even without force_cleanup_closed, we do not seem to be getting hangs in dvc-bench (tested with mnist dataset)
• connector_owner is not really required as the TCPConnector is closed anyway when the session is closed.

[skshetry]

• even without force_cleanup_closed, we do not seem to be getting hangs in dvc-bench (tested with mnist dataset)

I guess you meant enable_cleanup_closed? Let's keep it, we don't want to leak memory even for misconfigured servers (dvc can be used to get/import over arbitrary servers that our users don't control).
See cpython#73592, which is what I believe to be a motivation for the config to be introduced in aiohttp (see aiohttp#1568). This was fixed by introducing a new implementation of asyncio ssl in 3.11 (see cpython#88177 and cpython#31275).

[dtrifiro]

@skshetry re-added enable_cleanup_closed, moved most of the connector configuration to get_client

[efiop]

Looks like we should've pop()-ed ssl_verify here #40 Or not mixed client and connector kwargs at all.

[efiop]

Oh, looks like it is already fixed upstream.