From b4e0c75a302b785656c98241045f8c670fdf02ae Mon Sep 17 00:00:00 2001 From: Batuhan Taskaya Date: Wed, 8 Sep 2021 20:40:01 +0300 Subject: [PATCH] http: pass prepared filesystem arguments (#6565) --- dvc/fs/http.py | 21 +++++++++++++++++---- tests/unit/remote/test_http.py | 10 +++++----- 2 files changed, 22 insertions(+), 9 deletions(-) diff --git a/dvc/fs/http.py b/dvc/fs/http.py index c024100c4..0dce1f93c 100644 --- a/dvc/fs/http.py +++ b/dvc/fs/http.py @@ -50,7 +50,7 @@ class HTTPFileSystem(CallbackMixin, NoDirectoriesMixin, FSSpecWrapper): from dvc.config import ConfigError credentials = {} - client_args = credentials.setdefault("client_args", {}) + client_kwargs = credentials.setdefault("client_kwargs", {}) if config.get("auth"): user = config.get("user") @@ -68,7 +68,7 @@ class HTTPFileSystem(CallbackMixin, NoDirectoriesMixin, FSSpecWrapper): "'user' and 'password'" ) - client_args["auth"] = aiohttp.BasicAuth(user, password) + client_kwargs["auth"] = aiohttp.BasicAuth(user, password) elif auth_method == "custom": if custom_auth_header is None or password is None: raise ConfigError( @@ -83,7 +83,7 @@ class HTTPFileSystem(CallbackMixin, NoDirectoriesMixin, FSSpecWrapper): if "ssl_verify" in config: with fsspec_loop(): - client_args["connector"] = aiohttp.TCPConnector( + client_kwargs["connector"] = aiohttp.TCPConnector( ssl=make_context(config["ssl_verify"]) ) @@ -92,6 +92,7 @@ class HTTPFileSystem(CallbackMixin, NoDirectoriesMixin, FSSpecWrapper): return credentials async def get_client(self, **kwargs): + import aiohttp from aiohttp_retry import ExponentialRetry, RetryClient kwargs["retry_options"] = ExponentialRetry( @@ -100,6 +101,18 @@ class HTTPFileSystem(CallbackMixin, NoDirectoriesMixin, FSSpecWrapper): max_timeout=self.REQUEST_TIMEOUT, ) + # The default timeout for the aiohttp is 300 seconds + # which is too low for DVC's interactions (especially + # on the read) when dealing with large data blobs. We + # unlimit the total time to read, and only limit the + # time that is spent when connecting to the remote server. + kwargs["timeout"] = aiohttp.ClientTimeout( + total=None, + connect=self.REQUEST_TIMEOUT, + sock_connect=self.REQUEST_TIMEOUT, + sock_read=None, + ) + return RetryClient(**kwargs) @cached_property @@ -108,7 +121,7 @@ class HTTPFileSystem(CallbackMixin, NoDirectoriesMixin, FSSpecWrapper): HTTPFileSystem as _HTTPFileSystem, ) - return _HTTPFileSystem(timeout=self.REQUEST_TIMEOUT) + return _HTTPFileSystem(**self.fs_args) def _entry_hook(self, entry): entry["checksum"] = entry.get("ETag") or entry.get("Content-MD5") diff --git a/tests/unit/remote/test_http.py b/tests/unit/remote/test_http.py index d28ad381a..28ad284f5 100644 --- a/tests/unit/remote/test_http.py +++ b/tests/unit/remote/test_http.py @@ -23,7 +23,7 @@ def test_public_auth_method(dvc): fs = HTTPFileSystem(**config) - assert "auth" not in fs.fs_args["client_args"] + assert "auth" not in fs.fs_args["client_kwargs"] assert "headers" not in fs.fs_args @@ -40,8 +40,8 @@ def test_basic_auth_method(dvc): fs = HTTPFileSystem(**config) - assert fs.fs_args["client_args"]["auth"].login == user - assert fs.fs_args["client_args"]["auth"].password == password + assert fs.fs_args["client_kwargs"]["auth"].login == user + assert fs.fs_args["client_kwargs"]["auth"].password == password def test_custom_auth_method(dvc): @@ -70,7 +70,7 @@ def test_ssl_verify_disable(dvc): } fs = HTTPFileSystem(**config) - assert not fs.fs_args["client_args"]["connector"]._ssl + assert not fs.fs_args["client_kwargs"]["connector"]._ssl @patch("ssl.SSLContext.load_verify_locations") @@ -84,7 +84,7 @@ def test_ssl_verify_custom_cert(dvc, mocker): fs = HTTPFileSystem(**config) assert isinstance( - fs.fs_args["client_args"]["connector"]._ssl, ssl.SSLContext + fs.fs_args["client_kwargs"]["connector"]._ssl, ssl.SSLContext )