diff --git a/dvc/config.py b/dvc/config.py index 0fd312aa1..13965264b 100644 --- a/dvc/config.py +++ b/dvc/config.py @@ -252,7 +252,7 @@ class Config(object): # pylint: disable=too-many-instance-attributes def __init__(self, dvc_dir=None, validate=True): self.dvc_dir = dvc_dir - self.validate = validate + self.should_validate = validate if not dvc_dir: try: @@ -382,14 +382,10 @@ class Config(object): # pylint: disable=too-many-instance-attributes c = self._lower(c) self.config.merge(c) - if not self.validate: + if not self.should_validate: return - d = self.config.dict() - try: - d = self.COMPILED_SCHEMA(d) - except Invalid as exc: - raise ConfigError(str(exc)) from exc + d = self.validate() self.config = configobj.ConfigObj(d, write_empty_values=True) def save(self, config=None): @@ -427,6 +423,14 @@ class Config(object): # pylint: disable=too-many-instance-attributes raise config.write() + def validate(self, config=None): + d = (config or self.config).dict() + + try: + return self.COMPILED_SCHEMA(d) + except Invalid as exc: + raise ConfigError(str(exc)) from exc + def unset(self, section, opt=None, level=None, force=False): """Unsets specified option and/or section in the config. @@ -491,6 +495,8 @@ class Config(object): # pylint: disable=too-many-instance-attributes ) config[section][opt] = value + + self.validate(config) self.save(config) def get(self, section, opt=None, level=None): diff --git a/tests/func/test_config.py b/tests/func/test_config.py index f6cae78d3..e4581dd27 100644 --- a/tests/func/test_config.py +++ b/tests/func/test_config.py @@ -33,11 +33,11 @@ class TestConfigCLI(TestDvc): self.assertEqual(ret, 0) def _do_test(self, local=False): - section = "setsection" - field = "setfield" + section = "core" + field = "analytics" section_field = "{}.{}".format(section, field) - value = "setvalue" - newvalue = "setnewvalue" + value = "True" + newvalue = "False" base = ["config"] if local: diff --git a/tests/func/test_remote.py b/tests/func/test_remote.py index c6d1cffe9..2bbe5088f 100644 --- a/tests/func/test_remote.py +++ b/tests/func/test_remote.py @@ -32,7 +32,7 @@ class TestRemote(TestDvc): self.assertEqual(main(["remote", "list"]), 0) self.assertEqual( - main(["remote", "modify", remotes[0], "option", "value"]), 0 + main(["remote", "modify", remotes[0], "checksum_jobs", "1"]), 0 ) self.assertEqual(main(["remote", "remove", remotes[0]]), 0) @@ -113,7 +113,7 @@ class TestRemoteRemoveDefault(TestDvc): class TestRemoteRemove(TestDvc): def test(self): - ret = main(["config", "core.jobs", "1"]) + ret = main(["config", "core.checksum_jobs", "1"]) self.assertEqual(ret, 0) remote = "mys3" @@ -257,3 +257,11 @@ def test_modify_missing_remote(dvc): with pytest.raises(ConfigError, match=r"unable to find remote section"): remote_config.modify("myremote", "gdrive_client_id", "xxx") + + +def test_modify_invalid_key(dvc): + remote_config = RemoteConfig(dvc.config) + remote_config.add("example", "https://example.com") + + with pytest.raises(ConfigError, match=r"extra keys not allowed"): + remote_config.modify("example", "random_key", "value")