import: set core.remote when both --remote and --remote-config are sp…

…ecified From #9853 (comment)
iterative · Aug 17, 2023 · 1805edb · 1805edb
1 parent 1723abc
commit 1805edb
Show file tree

Hide file tree

Showing 3 changed files with 86 additions and 1 deletion.
diff --git a/dvc/dependency/repo.py b/dvc/dependency/repo.py
@@ -1,3 +1,4 @@
+from copy import deepcopy
 from typing import TYPE_CHECKING, Dict, Optional, Union
 
 from voluptuous import Any, Required
@@ -115,7 +116,7 @@ def _make_fs(
 
         conf = self.def_repo.get("config", {})
         if isinstance(conf, dict):
-            config = conf
+            config = deepcopy(conf)
         else:
             config = Config.load_file(conf)
 

diff --git a/dvc/repo/imp.py b/dvc/repo/imp.py
@@ -13,16 +13,28 @@ def imp(
     if rev is not None:
         erepo["rev"] = rev
 
+    if remote and remote_config and isinstance(config, str):
+        raise ValueError(
+            "Can't specify config path together with both remote and remote_config"
+        )
+
     if config is not None:
         erepo["config"] = config
 
     if remote is not None and remote_config is not None:
         conf = erepo.get("config") or {}
+
+        core = conf.get("core") or {}
+        core["remote"] = remote
+
         remotes = conf.get("remote") or {}
         remote_conf = remotes.get(remote) or {}
         remote_conf.update(remote_config)
         remotes[remote] = remote_conf
+
+        conf["core"] = core
         conf["remote"] = remotes
+
         erepo["config"] = conf
     elif remote is not None:
         erepo["remote"] = remote

diff --git a/tests/func/test_import.py b/tests/func/test_import.py
@@ -658,3 +658,75 @@ def test_parameterized_repo(tmp_dir, dvc, scm, erepo_dir, paths):
         "url": os.fspath(erepo_dir),
         "rev_lock": erepo_dir.scm.get_rev(),
     }
+
+
+@pytest.mark.parametrize(
+    "options, def_repo",
+    [
+        ({"config": "myconfig"}, {"config": "myconfig"}),
+        ({"remote": "myremote"}, {"remote": "myremote"}),
+        ({"remote_config": {"key": "value"}}, {"remote": {"key": "value"}}),
+        (
+            {
+                "remote": "myremote",
+                "remote_config": {"key": "value"},
+            },
+            {
+                "config": {
+                    "core": {"remote": "myremote"},
+                    "remote": {
+                        "myremote": {"key": "value"},
+                    },
+                },
+            },
+        ),
+        (
+            {
+                "remote": "myremote",
+                "remote_config": {"key": "value"},
+                "config": {"otherkey": "othervalue"},
+            },
+            {
+                "config": {
+                    "core": {"remote": "myremote"},
+                    "remote": {
+                        "myremote": {"key": "value"},
+                    },
+                    "otherkey": "othervalue",
+                },
+            },
+        ),
+    ],
+)
+def test_import_configs(tmp_dir, scm, dvc, erepo_dir, options, def_repo):
+    with erepo_dir.chdir():
+        erepo_dir.dvc_gen("foo", "foo content", commit="create foo")
+
+    (tmp_dir / "myconfig").touch()
+
+    stage = dvc.imp(
+        os.fspath(erepo_dir), "foo", "foo_imported", no_exec=True, **options
+    )
+    assert stage.deps[0].def_repo == {
+        "url": os.fspath(erepo_dir),
+        **def_repo,
+    }
+
+
+def test_import_invalid_configs(tmp_dir, scm, dvc, erepo_dir):
+    with erepo_dir.chdir():
+        erepo_dir.dvc_gen("foo", "foo content", commit="create foo")
+
+    with pytest.raises(
+        ValueError,
+        match="Can't specify config path together with both remote and remote_config",
+    ):
+        dvc.imp(
+            os.fspath(erepo_dir),
+            "foo",
+            "foo_imported",
+            no_exec=True,
+            config="myconfig",
+            remote="myremote",
+            remote_config={"key": "value"},
+        )