From 14e16d073ede3ad9f74c7def163457c16e017191 Mon Sep 17 00:00:00 2001 From: Michael Stegeman Date: Thu, 17 Dec 2020 10:59:51 -0900 Subject: [PATCH 1/5] cli: config: add support for --show-origin Support the --show-origin option for config, which, similar to git, prefixes each config option with the source file it originated from. Fixes #5119 --- dvc/command/config.py | 49 +++++++++++++++++++++++++++++++++++---- tests/func/test_config.py | 12 ++++++++++ 2 files changed, 57 insertions(+), 4 deletions(-) diff --git a/dvc/command/config.py b/dvc/command/config.py index ee2c07e430..604c89b149 100644 --- a/dvc/command/config.py +++ b/dvc/command/config.py @@ -1,8 +1,10 @@ import argparse import logging +import os from dvc.command.base import CmdBaseNoRepo, append_doc_link from dvc.config import Config, ConfigError +from dvc.repo import Repo from dvc.utils.flatten import flatten logger = logging.getLogger(__name__) @@ -34,6 +36,14 @@ def __init__(self, args): self.config = Config(validate=False) def run(self): + if self.args.show_origin: + if any((self.args.value, self.args.unset)): + logger.error( + "--show-origin can't be used together with any of these " + "options: -u/--unset, value" + ) + return 1 + if self.args.list: if any((self.args.name, self.args.value, self.args.unset)): logger.error( @@ -43,7 +53,13 @@ def run(self): return 1 conf = self.config.load_one(self.args.level) - logger.info("\n".join(self._format_config(conf))) + prefix = self._config_file_prefix( + self.args.show_origin, + self.config, + self.args.level + ) + + logger.info("\n".join(self._format_config(conf, prefix))) return 0 if self.args.name is None: @@ -54,10 +70,16 @@ def run(self): if self.args.value is None and not self.args.unset: conf = self.config.load_one(self.args.level) + prefix = self._config_file_prefix( + self.args.show_origin, + self.config, + self.args.level + ) + if remote: conf = conf["remote"] self._check(conf, remote, section, opt) - logger.info(conf[section][opt]) + logger.info("{}{}".format(prefix, conf[section][opt])) return 0 with self.config.edit(self.args.level) as conf: @@ -90,9 +112,22 @@ def _check(self, conf, remote, section, opt=None): ) @staticmethod - def _format_config(config): + def _format_config(config, prefix): for key, value in flatten(config).items(): - yield f"{key}={value}" + yield f"{prefix}{key}={value}" + + @staticmethod + def _config_file_prefix(show_origin, config, level): + if not show_origin: + return '' + + fname = config.files[level] + + if level in ['local', 'repo']: + common = os.path.commonprefix([fname, Repo.find_root()]) + fname = fname[len(common) + 1:] + + return fname + '\t' parent_config_parser = argparse.ArgumentParser(add_help=False) @@ -152,4 +187,10 @@ def add_parser(subparsers, parent_parser): action="store_true", help="List all defined config values.", ) + config_parser.add_argument( + "--show-origin", + default=False, + action="store_true", + help="Show the source file containing each config value." + ) config_parser.set_defaults(func=CmdConfig) diff --git a/tests/func/test_config.py b/tests/func/test_config.py index 0e0d6bb01d..f55862284e 100644 --- a/tests/func/test_config.py +++ b/tests/func/test_config.py @@ -48,9 +48,15 @@ def _do_test(self, local=False): self.assertEqual(ret, 0) self.assertTrue(self._contains(section, field, value, local)) + ret = main(base + [section_field, value, "--show-origin"]) + self.assertEqual(ret, 1) + ret = main(base + [section_field]) self.assertEqual(ret, 0) + ret = main(base + ["--show-origin", section_field]) + self.assertEqual(ret, 0) + ret = main(base + [section_field, newvalue]) self.assertEqual(ret, 0) self.assertTrue(self._contains(section, field, newvalue, local)) @@ -60,9 +66,15 @@ def _do_test(self, local=False): self.assertEqual(ret, 0) self.assertFalse(self._contains(section, field, value, local)) + ret = main(base + [section_field, "--unset", "--show-origin"]) + self.assertEqual(ret, 1) + ret = main(base + ["--list"]) self.assertEqual(ret, 0) + ret = main(base + ["--list", "--show-origin"]) + self.assertEqual(ret, 0) + def test(self): self._do_test(False) From 40b1d8c831e00646e6867e8cd76ba9eedc6b3dcd Mon Sep 17 00:00:00 2001 From: Michael Stegeman Date: Thu, 17 Dec 2020 11:18:01 -0900 Subject: [PATCH 2/5] Reformat with black. --- dvc/command/config.py | 20 ++++++++------------ 1 file changed, 8 insertions(+), 12 deletions(-) diff --git a/dvc/command/config.py b/dvc/command/config.py index 604c89b149..eff3dc81eb 100644 --- a/dvc/command/config.py +++ b/dvc/command/config.py @@ -54,9 +54,7 @@ def run(self): conf = self.config.load_one(self.args.level) prefix = self._config_file_prefix( - self.args.show_origin, - self.config, - self.args.level + self.args.show_origin, self.config, self.args.level ) logger.info("\n".join(self._format_config(conf, prefix))) @@ -71,9 +69,7 @@ def run(self): if self.args.value is None and not self.args.unset: conf = self.config.load_one(self.args.level) prefix = self._config_file_prefix( - self.args.show_origin, - self.config, - self.args.level + self.args.show_origin, self.config, self.args.level ) if remote: @@ -112,22 +108,22 @@ def _check(self, conf, remote, section, opt=None): ) @staticmethod - def _format_config(config, prefix): + def _format_config(config, prefix=""): for key, value in flatten(config).items(): yield f"{prefix}{key}={value}" @staticmethod def _config_file_prefix(show_origin, config, level): if not show_origin: - return '' + return "" fname = config.files[level] - if level in ['local', 'repo']: + if level in ["local", "repo"]: common = os.path.commonprefix([fname, Repo.find_root()]) - fname = fname[len(common) + 1:] + fname = fname[len(common) + 1 :] - return fname + '\t' + return fname + "\t" parent_config_parser = argparse.ArgumentParser(add_help=False) @@ -191,6 +187,6 @@ def add_parser(subparsers, parent_parser): "--show-origin", default=False, action="store_true", - help="Show the source file containing each config value." + help="Show the source file containing each config value.", ) config_parser.set_defaults(func=CmdConfig) From d7238e9aa48cb3b8326276cace9c55cb071a19f1 Mon Sep 17 00:00:00 2001 From: Michael Stegeman Date: Fri, 18 Dec 2020 09:00:15 -0900 Subject: [PATCH 3/5] Simplify logic for local config files. --- dvc/command/config.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/dvc/command/config.py b/dvc/command/config.py index eff3dc81eb..70deb90f50 100644 --- a/dvc/command/config.py +++ b/dvc/command/config.py @@ -120,8 +120,7 @@ def _config_file_prefix(show_origin, config, level): fname = config.files[level] if level in ["local", "repo"]: - common = os.path.commonprefix([fname, Repo.find_root()]) - fname = fname[len(common) + 1 :] + fname = os.path.relpath(fname, start=Repo.find_root()) return fname + "\t" From 87155e7bf1d5bd9c4fbbf5c8db53d5bbc50f6077 Mon Sep 17 00:00:00 2001 From: Michael Stegeman Date: Tue, 29 Dec 2020 22:52:02 -0900 Subject: [PATCH 4/5] Add test for --show-config output. --- tests/func/test_config.py | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/tests/func/test_config.py b/tests/func/test_config.py index f55862284e..ee603b0096 100644 --- a/tests/func/test_config.py +++ b/tests/func/test_config.py @@ -186,3 +186,28 @@ def test_config_remote(tmp_dir, dvc, caplog): caplog.clear() assert main(["config", "remote.myremote.region"]) == 0 assert "myregion" in caplog.text + + +def test_config_list(tmp_dir, dvc, caplog): + (tmp_dir / ".dvc" / "config").write_text( + "['remote \"myremote\"']\n" + " url = s3://bucket/path\n" + " region = myregion\n" + ) + + caplog.clear() + assert main(["config", "--show-origin", "remote.myremote.url"]) == 0 + assert ( + "{}\t{}\n".format(os.path.join(".dvc", "config"), "s3://bucket/path") + in caplog.text + ) + + caplog.clear() + assert main(["config", "--list", "--show-origin"]) == 0 + assert ( + "{}\t{}\n".format( + os.path.join(".dvc", "config"), + "remote.myremote.url=s3://bucket/path", + ) + in caplog.text + ) From e6def3721d7df2f77e5f57dfd66c5b7af24ec97b Mon Sep 17 00:00:00 2001 From: Ruslan Kuprieiev Date: Wed, 30 Dec 2020 21:25:27 +0200 Subject: [PATCH 5/5] Update tests/func/test_config.py --- tests/func/test_config.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/func/test_config.py b/tests/func/test_config.py index ee603b0096..a208cbcdaf 100644 --- a/tests/func/test_config.py +++ b/tests/func/test_config.py @@ -188,7 +188,7 @@ def test_config_remote(tmp_dir, dvc, caplog): assert "myregion" in caplog.text -def test_config_list(tmp_dir, dvc, caplog): +def test_config_show_origin(tmp_dir, dvc, caplog): (tmp_dir / ".dvc" / "config").write_text( "['remote \"myremote\"']\n" " url = s3://bucket/path\n"