Repository: iterative/dvc

Issue:
core.checksum_jobs doesn't work for remotes
https://github.com/iterative/dvc/issues/2891#issuecomment-562812250


Discussion:
mroutis: Thanks for the PR, @JIoJIaJIu, left some comments :)

Is your suggestion about using `dvc.config.settings` instead of `dvc.config.config`? 
JIoJIaJIu: > Thanks for the PR, @JIoJIaJIu, left some comments :)
> 
> Is your suggestion about renaming `dvc.config.config` to `dvc.config.settings`?

Hello @mroutis 
No, I meant the part when some component is initialized with partial of the `dvc.config`
Here for example it has names as a **settings** - https://github.com/iterative/dvc/blob/fe635a5040c9d593cd1bdef3fa31f4df7af85259/dvc/remote/__init__.py#L42
But further in **RemoteBASE** it has naming as `config` that introduces ambiguity - https://github.com/iterative/dvc/blob/fe635a5040c9d593cd1bdef3fa31f4df7af85259/dvc/remote/base.py#L85

I suggest just to define such terms as
```
# whole config
...
[core]  # specific section - core
# everything under section are settings
checksum_jobs = 100

['remote "aws"'] # specific section - remote aws
# everything under section are settings
url = s3://bucket/name 
...
```

And change https://github.com/iterative/dvc/blob/fe635a5040c9d593cd1bdef3fa31f4df7af85259/dvc/remote/base.py#L85 to
```
def __init__(self, repo, settings):
```
So it means that component (**Remote**) can be initialized with settings which have priority1 and use config if needs as fallback rather then use terms 2 configs where the one is a child of the second
What do you think? 
efiop: @JIoJIaJIu Looks good!

Good point about `settings`! Could you please create an issue for that? 

Patch:
diff --git a/dvc/config.py b/dvc/config.py
index ad2070939..0fd312aa1 100644
--- a/dvc/config.py
+++ b/dvc/config.py
@@ -186,6 +186,7 @@ class Config(object):  # pylint: disable=too-many-instance-attributes
     SECTION_GDRIVE_CLIENT_SECRET = "gdrive_client_secret"
     SECTION_GDRIVE_USER_CREDENTIALS_FILE = "gdrive_user_credentials_file"
 
+    SECTION_REMOTE_CHECKSUM_JOBS = "checksum_jobs"
     SECTION_REMOTE_REGEX = r'^\s*remote\s*"(?P<name>.*)"\s*$'
     SECTION_REMOTE_FMT = 'remote "{}"'
     SECTION_REMOTE_URL = "url"
@@ -214,6 +215,7 @@ class Config(object):  # pylint: disable=too-many-instance-attributes
         SECTION_GCP_PROJECTNAME: str,
         SECTION_CACHE_TYPE: supported_cache_type,
         Optional(SECTION_CACHE_PROTECTED, default=False): Bool,
+        SECTION_REMOTE_CHECKSUM_JOBS: All(Coerce(int), Range(1)),
         SECTION_REMOTE_USER: str,
         SECTION_REMOTE_PORT: Coerce(int),
         SECTION_REMOTE_KEY_FILE: str,
diff --git a/dvc/remote/base.py b/dvc/remote/base.py
index 36409645e..9f47779c8 100644
--- a/dvc/remote/base.py
+++ b/dvc/remote/base.py
@@ -86,11 +86,7 @@ class RemoteBASE(object):
         self.repo = repo
 
         self._check_requires(config)
-
-        core = config.get(Config.SECTION_CORE, {})
-        self.checksum_jobs = core.get(
-            Config.SECTION_CORE_CHECKSUM_JOBS, self.CHECKSUM_JOBS
-        )
+        self.checksum_jobs = self._get_checksum_jobs(config)
 
         self.protected = False
         self.no_traverse = config.get(Config.SECTION_REMOTE_NO_TRAVERSE, True)
@@ -142,6 +138,19 @@ class RemoteBASE(object):
         ).format(url, missing, " ".join(missing), self.scheme)
         raise RemoteMissingDepsError(msg)
 
+    def _get_checksum_jobs(self, config):
+        checksum_jobs = config.get(Config.SECTION_REMOTE_CHECKSUM_JOBS)
+        if checksum_jobs:
+            return checksum_jobs
+
+        if self.repo:
+            core = self.repo.config.config.get(Config.SECTION_CORE, {})
+            return core.get(
+                Config.SECTION_CORE_CHECKSUM_JOBS, self.CHECKSUM_JOBS
+            )
+
+        return self.CHECKSUM_JOBS
+
     def __repr__(self):
         return "{class_name}: '{path_info}'".format(
             class_name=type(self).__name__,


Test patch:
diff --git a/tests/unit/remote/test_gdrive.py b/tests/unit/remote/test_gdrive.py
index bdc895107..09dc7e62b 100644
--- a/tests/unit/remote/test_gdrive.py
+++ b/tests/unit/remote/test_gdrive.py
@@ -1,6 +1,7 @@
 import pytest
 import os
 
+from dvc.config import Config
 from dvc.remote.gdrive import (
     RemoteGDrive,
     GDriveAccessTokenRefreshError,
@@ -14,6 +15,7 @@ USER_CREDS_MISSED_KEY_ERROR = "{}"
 
 class Repo(object):
     tmp_dir = ""
+    config = Config()
 
 
 class TestRemoteGDrive(object):
diff --git a/tests/unit/remote/test_remote.py b/tests/unit/remote/test_remote.py
new file mode 100644
index 000000000..b2fa73aff
--- /dev/null
+++ b/tests/unit/remote/test_remote.py
@@ -0,0 +1,41 @@
+from dvc.remote import Remote
+
+
+def set_config_opts(dvc, commands):
+    list(map(lambda args: dvc.config.set(*args), commands))
+
+
+def test_remote_with_checksum_jobs(dvc):
+    set_config_opts(
+        dvc,
+        [
+            ('remote "with_checksum_jobs"', "url", "s3://bucket/name"),
+            ('remote "with_checksum_jobs"', "checksum_jobs", 100),
+            ("core", "checksum_jobs", 200),
+        ],
+    )
+
+    remote = Remote(dvc, name="with_checksum_jobs")
+    assert remote.checksum_jobs == 100
+
+
+def test_remote_without_checksum_jobs(dvc):
+    set_config_opts(
+        dvc,
+        [
+            ('remote "without_checksum_jobs"', "url", "s3://bucket/name"),
+            ("core", "checksum_jobs", "200"),
+        ],
+    )
+
+    remote = Remote(dvc, name="without_checksum_jobs")
+    assert remote.checksum_jobs == 200
+
+
+def test_remote_without_checksum_jobs_default(dvc):
+    set_config_opts(
+        dvc, [('remote "without_checksum_jobs"', "url", "s3://bucket/name")]
+    )
+
+    remote = Remote(dvc, name="without_checksum_jobs")
+    assert remote.checksum_jobs == remote.CHECKSUM_JOBS