Repository: iterative/dvc

Issue:
project: formalize extra dependency types
While working on #6540, I noticed that it would be a bit simpler if we had dependencies organized as sole files instead of the current way (so i can simply strip the version specifiers out on a certain directory to test with the newer versions). A simple way of doing this is just creating a `requirements/` directory and adding entries for each remote we have as a new file. This will also allow dependabot to upgrade our remote dependencies automatically (just like it is doing for the tests).

Discussion:

Patch:
diff --git a/MANIFEST.in b/MANIFEST.in
index 226626ef5..5099b94cc 100644
--- a/MANIFEST.in
+++ b/MANIFEST.in
@@ -1,3 +1,3 @@
 include fastentrypoints.py
 include LICENSE
-include test_requirements.txt
+recursive-include requirements *.txt
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/dvc/fs/ssh.py b/dvc/fs/ssh.py
index 23f15f888..1947b3d6f 100644
--- a/dvc/fs/ssh.py
+++ b/dvc/fs/ssh.py
@@ -67,6 +67,7 @@ class SSHFileSystem(CallbackMixin, FSSpecWrapper):
         )
 
         login_info["password"] = config.get("password")
+        login_info["passphrase"] = config.get("password")
 
         raw_keys = []
         if config.get("keyfile"):
diff --git a/dvc/state.py b/dvc/state.py
index 5b2f55d0a..4881a62ef 100644
--- a/dvc/state.py
+++ b/dvc/state.py
@@ -53,7 +53,6 @@ class State(StateBase):  # pylint: disable=too-many-instance-attributes
         self.tmp_dir = tmp_dir
         self.root_dir = root_dir
         self.dvcignore = dvcignore
-        self.fs = LocalFileSystem()
 
         if not tmp_dir:
             return
@@ -85,7 +84,7 @@ class State(StateBase):  # pylint: disable=too-many-instance-attributes
         assert isinstance(hash_info, HashInfo)
         assert os.path.exists(path_info)
 
-        mtime, size = get_mtime_and_size(path_info, self.fs, self.dvcignore)
+        mtime, size = get_mtime_and_size(path_info, fs, self.dvcignore)
         inode = get_inode(path_info)
 
         logger.debug(
@@ -121,7 +120,7 @@ class State(StateBase):  # pylint: disable=too-many-instance-attributes
         if not os.path.exists(path):
             return None
 
-        mtime, size = get_mtime_and_size(path, self.fs, self.dvcignore)
+        mtime, size = get_mtime_and_size(path, fs, self.dvcignore)
         inode = get_inode(path)
 
         value = self.md5s.get(inode)
@@ -143,10 +142,10 @@ class State(StateBase):  # pylint: disable=too-many-instance-attributes
 
         assert isinstance(path_info, str) or path_info.scheme == "local"
 
-        if not self.fs.exists(path_info):
+        if not fs.exists(path_info):
             return
 
-        mtime, _ = get_mtime_and_size(path_info, self.fs, self.dvcignore)
+        mtime, _ = get_mtime_and_size(path_info, fs, self.dvcignore)
         inode = get_inode(path_info)
         relative_path = relpath(path_info, self.root_dir)
 
@@ -168,11 +167,11 @@ class State(StateBase):  # pylint: disable=too-many-instance-attributes
             for relative_path in ref:
                 path = os.path.join(self.root_dir, relative_path)
 
-                if path in used or not self.fs.exists(path):
+                if path in used or not fs.exists(path):
                     continue
 
                 inode = get_inode(path)
-                mtime, _ = get_mtime_and_size(path, self.fs, self.dvcignore)
+                mtime, _ = get_mtime_and_size(path, fs, self.dvcignore)
 
                 if ref[relative_path] == (inode, mtime):
                     logger.debug("Removing '%s' as unused link.", path)
diff --git a/dvc/version.py b/dvc/version.py
index 1edcac951..aa2319ac1 100644
--- a/dvc/version.py
+++ b/dvc/version.py
@@ -6,7 +6,7 @@
 import os
 import subprocess
 
-_BASE_VERSION = "2.7.1"
+_BASE_VERSION = "2.7.2"
 
 
 def _generate_version(base_version):
diff --git a/requirements/azure.txt b/requirements/azure.txt
new file mode 100644
index 000000000..1bd6d816d
--- /dev/null
+++ b/requirements/azure.txt
@@ -0,0 +1,3 @@
+adlfs==2021.8.2
+azure-identity>=1.4.0
+knack
diff --git a/requirements/gdrive.txt b/requirements/gdrive.txt
new file mode 100644
index 000000000..e5e8ee184
--- /dev/null
+++ b/requirements/gdrive.txt
@@ -0,0 +1,1 @@
+pydrive2[fsspec]>=1.9.2
diff --git a/requirements/gs.txt b/requirements/gs.txt
new file mode 100644
index 000000000..4616d9a0f
--- /dev/null
+++ b/requirements/gs.txt
@@ -0,0 +1,1 @@
+gcsfs==2021.8.1
diff --git a/requirements/hdfs.txt b/requirements/hdfs.txt
new file mode 100644
index 000000000..06b8ec737
--- /dev/null
+++ b/requirements/hdfs.txt
@@ -0,0 +1,1 @@
+pyarrow>=2.0.0; python_version < '3.10'
diff --git a/requirements/oss.txt b/requirements/oss.txt
new file mode 100644
index 000000000..92e507ee2
--- /dev/null
+++ b/requirements/oss.txt
@@ -0,0 +1,1 @@
+ossfs==2021.8.0
diff --git a/requirements/s3.txt b/requirements/s3.txt
new file mode 100644
index 000000000..495ee8285
--- /dev/null
+++ b/requirements/s3.txt
@@ -0,0 +1,2 @@
+s3fs==2021.8.1
+aiobotocore[boto3]>1.0.1
diff --git a/requirements/ssh.txt b/requirements/ssh.txt
new file mode 100644
index 000000000..c6238d7b3
--- /dev/null
+++ b/requirements/ssh.txt
@@ -0,0 +1,1 @@
+sshfs>=2021.8.1
diff --git a/requirements/ssh_gssapi.txt b/requirements/ssh_gssapi.txt
new file mode 100644
index 000000000..6345c0896
--- /dev/null
+++ b/requirements/ssh_gssapi.txt
@@ -0,0 +1,1 @@
+sshfs[gssapi]>=2021.8.1
diff --git a/requirements/terraform.txt b/requirements/terraform.txt
new file mode 100644
index 000000000..a7bbfa0e7
--- /dev/null
+++ b/requirements/terraform.txt
@@ -0,0 +1,1 @@
+tpi[ssh]>=0.0.0
diff --git a/test_requirements.txt b/requirements/tests.txt
similarity index 100%
rename from test_requirements.txt
rename to requirements/tests.txt
diff --git a/requirements/webdav.txt b/requirements/webdav.txt
new file mode 100644
index 000000000..e5c592334
--- /dev/null
+++ b/requirements/webdav.txt
@@ -0,0 +1,1 @@
+webdav4>=0.9.1
diff --git a/requirements/webhdfs.txt b/requirements/webhdfs.txt
new file mode 100644
index 000000000..bf50d1a6a
--- /dev/null
+++ b/requirements/webhdfs.txt
@@ -0,0 +1,1 @@
+hdfs==2.5.8
diff --git a/setup.py b/setup.py
index bede25d4a..294ce3765 100644
--- a/setup.py
+++ b/setup.py
@@ -97,29 +97,22 @@ install_requires = [
 
 
 # Extra dependencies for remote integrations
+extra_requirements = {
+    path.stem: path.read_text().strip().splitlines()
+    for path in Path("requirements").glob("*.txt")
+}
 
-gs = ["gcsfs==2021.8.1"]
-gdrive = ["pydrive2[fsspec]>=1.9.2"]
-s3 = ["s3fs==2021.8.1", "aiobotocore[boto3]>1.0.1"]
-azure = ["adlfs==2021.8.2", "azure-identity>=1.4.0", "knack"]
-oss = ["ossfs==2021.8.0"]
-ssh = ["sshfs>=2021.8.1"]
-
-hdfs = ["pyarrow>=2.0.0; python_version < '3.10'"]
-webhdfs = ["hdfs==2.5.8"]
-webdav = ["webdav4>=0.9.1"]
 # gssapi should not be included in all_remotes, because it doesn't have wheels
 # for linux and mac, so it will fail to compile if user doesn't have all the
 # requirements, including kerberos itself. Once all the wheels are available,
 # we can start shipping it by default.
-ssh_gssapi = ["sshfs[gssapi]>=2021.8.1"]
-all_remotes = gs + s3 + azure + ssh + oss + gdrive + hdfs + webhdfs + webdav
-
-terraform = ["tpi[ssh]>=0.0.0"]
 
-tests_requirements = (
-    Path("test_requirements.txt").read_text().strip().splitlines()
-) + terraform
+extra_requirements["all"] = [
+    requirements
+    for key, requirements in extra_requirements.items()
+    if key not in ("tests", "ssh_gssapi", "terraform")
+]
+extra_requirements["tests"] += extra_requirements["terraform"]
 
 setup(
     name="dvc",
@@ -133,21 +126,7 @@ setup(
     download_url="https://github.com/iterative/dvc",
     license="Apache License 2.0",
     install_requires=install_requires,
-    extras_require={
-        "all": all_remotes,
-        "gs": gs,
-        "gdrive": gdrive,
-        "s3": s3,
-        "azure": azure,
-        "oss": oss,
-        "ssh": ssh,
-        "ssh_gssapi": ssh_gssapi,
-        "hdfs": hdfs,
-        "webhdfs": webhdfs,
-        "webdav": webdav,
-        "terraform": terraform,
-        "tests": tests_requirements,
-    },
+    extras_require=extra_requirements,
     keywords="data-science data-version-control machine-learning git"
     " developer-tools reproducibility collaboration ai",
     python_requires=">=3.6",


Test patch:
diff --git a/tests/unit/fs/test_ssh.py b/tests/unit/fs/test_ssh.py
index da5a544e6..a7372ddd6 100644
--- a/tests/unit/fs/test_ssh.py
+++ b/tests/unit/fs/test_ssh.py
@@ -34,10 +34,13 @@ def test_get_kwargs_from_urls():
 
 
 def test_init():
-    fs = SSHFileSystem(user="test", host="12.34.56.78", port="1234")
+    fs = SSHFileSystem(
+        user="test", host="12.34.56.78", port="1234", password="xxx"
+    )
     assert fs.fs_args["username"] == "test"
     assert fs.fs_args["host"] == "12.34.56.78"
     assert fs.fs_args["port"] == "1234"
+    assert fs.fs_args["password"] == fs.fs_args["passphrase"] == "xxx"
 
 
 mock_ssh_config = """
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
     )