remote: allow local options for repo remotes (#4309)

* remote: allow local options for repo remotes Fixes #4276 * Update tests/func/test_remote.py Co-authored-by: Saugat Pachhai <[email protected]> * add more tests Co-authored-by: Saugat Pachhai <[email protected]>
iterative · Jul 31, 2020 · ed1c3a7 · ed1c3a7
1 parent fcf2e35
commit ed1c3a7
Show file tree

Hide file tree

Showing 3 changed files with 29 additions and 6 deletions.
diff --git a/dvc/command/remote.py b/dvc/command/remote.py
@@ -3,7 +3,7 @@
 
 from dvc.command.base import append_doc_link, fix_subparsers
 from dvc.command.config import CmdConfig
-from dvc.config import ConfigError
+from dvc.config import ConfigError, merge
 from dvc.utils import format_link
 
 logger = logging.getLogger(__name__)
@@ -61,7 +61,12 @@ def run(self):
 class CmdRemoteModify(CmdRemote):
     def run(self):
         with self.config.edit(self.args.level) as conf:
-            self._check_exists(conf)
+            merged = self.config.load_config_to_level(self.args.level)
+            merge(merged, conf)
+            self._check_exists(merged)
+
+            if self.args.name not in conf["remote"]:
+                conf["remote"][self.args.name] = {}
             section = conf["remote"][self.args.name]
             if self.args.unset:
                 section.pop(self.args.option, None)

diff --git a/dvc/config.py b/dvc/config.py
@@ -400,7 +400,7 @@ def load_config_to_level(self, level=None):
             if merge_level == level:
                 break
             if merge_level in self.files:
-                _merge(merged_conf, self.load_one(merge_level))
+                merge(merged_conf, self.load_one(merge_level))
         return merged_conf
 
     @contextmanager
@@ -414,7 +414,7 @@ def edit(self, level="repo"):
         conf = self._save_paths(conf, self.files[level])
 
         merged_conf = self.load_config_to_level(level)
-        _merge(merged_conf, conf)
+        merge(merged_conf, conf)
         self.validate(merged_conf)
 
         self._save_config(level, conf)
@@ -453,11 +453,11 @@ def _pack_remotes(conf):
     return result
 
 
-def _merge(into, update):
+def merge(into, update):
     """Merges second dict into first recursively"""
     for key, val in update.items():
         if isinstance(into.get(key), dict) and isinstance(val, dict):
-            _merge(into[key], val)
+            merge(into[key], val)
         else:
             into[key] = val
 

diff --git a/tests/func/test_remote.py b/tests/func/test_remote.py
@@ -232,6 +232,24 @@ def test_modify_missing_remote(tmp_dir, dvc):
     assert main(["remote", "modify", "myremote", "user", "xxx"]) == 251
 
 
+def test_remote_modify_local_on_repo_config(tmp_dir, dvc):
+    assert main(["remote", "add", "myremote", "http://example.com/path"]) == 0
+    assert (
+        main(["remote", "modify", "myremote", "user", "xxx", "--local"]) == 0
+    )
+    assert dvc.config.load_one("local")["remote"]["myremote"] == {
+        "user": "xxx"
+    }
+    assert dvc.config.load_one("repo")["remote"]["myremote"] == {
+        "url": "http://example.com/path"
+    }
+    dvc.config.load()
+    assert dvc.config["remote"]["myremote"] == {
+        "url": "http://example.com/path",
+        "user": "xxx",
+    }
+
+
 def test_external_dir_resource_on_no_cache(tmp_dir, dvc, tmp_path_factory):
     # https://github.com/iterative/dvc/issues/2647, is some situations
     # (external dir dependency) cache is required to calculate dir md5