diff --git a/dvc/config.py b/dvc/config.py index 6e1ccd7f5f..1fc63acbb3 100644 --- a/dvc/config.py +++ b/dvc/config.py @@ -278,10 +278,7 @@ def load(self, validate=True): Raises: ConfigError: thrown if config has an invalid format. """ - conf = {} - for level in self.LEVELS: - if level in self.files: - _merge(conf, self.load_one(level)) + conf = self._load_config_to_level() if validate: conf = self.validate(conf) @@ -333,6 +330,15 @@ def _map_dirs(conf, func): dirs_schema = {"cache": {"dir": func}, "remote": {str: {"url": func}}} return Schema(dirs_schema, extra=ALLOW_EXTRA)(conf) + def _load_config_to_level(self, level=None): + merged_conf = {} + for merge_level in self.LEVELS: + if merge_level == level: + break + if merge_level in self.files: + _merge(merged_conf, self.load_one(merge_level)) + return merged_conf + @contextmanager def edit(self, level="repo"): if level in {"repo", "local"} and self.dvc_dir is None: @@ -342,6 +348,11 @@ def edit(self, level="repo"): yield conf conf = self._save_paths(conf, self.files[level]) + + merged_conf = self._load_config_to_level(level) + _merge(merged_conf, conf) + self.validate(merged_conf) + _save_config(self.files[level], conf) self.load() diff --git a/tests/func/test_config.py b/tests/func/test_config.py index 1ae0118b77..9f0293dc33 100644 --- a/tests/func/test_config.py +++ b/tests/func/test_config.py @@ -94,6 +94,12 @@ def test_merging_two_levels(dvc): with dvc.config.edit() as conf: conf["remote"]["test"] = {"url": "ssh://example.com"} + with pytest.raises( + ConfigError, match=r"expected 'url' for dictionary value" + ): + with dvc.config.edit("global") as conf: + conf["remote"]["test"] = {"password": "1"} + with dvc.config.edit("local") as conf: conf["remote"]["test"] = {"password": "1"} diff --git a/tests/func/test_remote.py b/tests/func/test_remote.py index f6f37017ea..4369203dd7 100644 --- a/tests/func/test_remote.py +++ b/tests/func/test_remote.py @@ -254,3 +254,19 @@ def test_push_order(tmp_dir, dvc, tmp_path_factory, mocker): dvc.push() # last uploaded file should be dir checksum assert mocked_upload.call_args[0][0].endswith(".dir") + + +def test_remote_modify_validation(dvc): + remote_name = "drive" + unsupported_config = "unsupported_config" + assert ( + main(["remote", "add", "-d", remote_name, "gdrive://test/test"]) == 0 + ) + assert ( + main( + ["remote", "modify", remote_name, unsupported_config, "something"] + ) + == 251 + ) + config = configobj.ConfigObj(dvc.config.files["repo"]) + assert unsupported_config not in config['remote "{}"'.format(remote_name)]