From 39d5d36033fa790249d9b5012df427c6fb54e3ca Mon Sep 17 00:00:00 2001
From: Amey Kulkarni <29138700+ameyk-2409@users.noreply.github.com>
Date: Wed, 15 Sep 2021 11:46:23 +0530
Subject: [PATCH] dvc cli: fixed error in 'dvc config --list' in non repo dir
 (#6587)

* dvc cli: fixed error in 'dvc config --list' in non repo dir

Fixes #6496

* Issue 6496: Added test that covers the issue

* dvc cli: fixes error in 'dvc config <name>' in non dvc repo dir

Fixes the specified error. Did some refactoring based on previous commits.
Also added a test case for setting config in non dvc repo dir.

Fixes #6496

* Issue-6496: dvc config --<level> and dvc config --list --<level> will now throw config error if ran in non dvc repo with repo only level

* Issue 6496: Segregated config levels into repo and system levels that helped in simplifying the logic to decide appropriate level at the time of cli operation.
---
 dvc/command/config.py     | 21 +++++++++++++++++----
 dvc/config.py             |  6 ++++--
 tests/func/test_config.py | 36 ++++++++++++++++++++++++++++++++++++
 3 files changed, 57 insertions(+), 6 deletions(-)

diff --git a/dvc/command/config.py b/dvc/command/config.py
index aee59cf75..a3c76009c 100644
--- a/dvc/command/config.py
+++ b/dvc/command/config.py
@@ -66,7 +66,8 @@ class CmdConfig(CmdBaseNoRepo):
             )
             return 1
 
-        levels = [self.args.level] if self.args.level else self.config.LEVELS
+        levels = self._get_appropriate_levels(self.args.level)
+
         for level in levels:
             conf = self.config.read(level)
             prefix = self._config_file_prefix(
@@ -81,9 +82,7 @@ class CmdConfig(CmdBaseNoRepo):
     def _get(self, remote, section, opt):
         from dvc.config import ConfigError
 
-        levels = (
-            [self.args.level] if self.args.level else self.config.LEVELS[::-1]
-        )
+        levels = self._get_appropriate_levels(self.args.level)[::-1]
 
         for level in levels:
             conf = self.config.read(level)
@@ -136,6 +135,20 @@ class CmdConfig(CmdBaseNoRepo):
                 f"option '{opt}' doesn't exist in {name} '{section}'"
             )
 
+    def _get_appropriate_levels(self, levels):
+        if levels:
+            self._validate_level_for_non_repo_operation(levels)
+            return [levels]
+        if self.config.dvc_dir is None:
+            return self.config.SYSTEM_LEVELS
+        return self.config.LEVELS
+
+    def _validate_level_for_non_repo_operation(self, level):
+        from dvc.config import ConfigError
+
+        if self.config.dvc_dir is None and level in self.config.REPO_LEVELS:
+            raise ConfigError("Not inside a DVC repo")
+
     @staticmethod
     def _format_config(config, prefix=""):
         from dvc.utils.flatten import flatten
diff --git a/dvc/config.py b/dvc/config.py
index 98804bf22..064b839ae 100644
--- a/dvc/config.py
+++ b/dvc/config.py
@@ -76,8 +76,10 @@ class Config(dict):
     APPNAME = "dvc"
     APPAUTHOR = "iterative"
 
+    SYSTEM_LEVELS = ("system", "global")
+    REPO_LEVELS = ("repo", "local")
     # In the order they shadow each other
-    LEVELS = ("system", "global", "repo", "local")
+    LEVELS = SYSTEM_LEVELS + REPO_LEVELS
 
     CONFIG = "config"
     CONFIG_LOCAL = "config.local"
@@ -286,7 +288,7 @@ class Config(dict):
     def edit(self, level=None, validate=True):
         # NOTE: we write to repo config by default, same as git config
         level = level or "repo"
-        if level in {"repo", "local"} and self.dvc_dir is None:
+        if self.dvc_dir is None and level in self.REPO_LEVELS:
             raise ConfigError("Not inside a DVC repo")
 
         conf = self.load_one(level)
diff --git a/tests/func/test_config.py b/tests/func/test_config.py
index 9dbf9c86c..c9362093c 100644
--- a/tests/func/test_config.py
+++ b/tests/func/test_config.py
@@ -77,6 +77,12 @@ def test_config_set_local(tmp_dir, dvc):
     assert (tmp_dir / ".dvc" / "config.local").read_text() == "\n"
 
 
+def test_config_set_in_non_dvc_repo(tmp_dir, caplog):
+    assert main(["config", "core.analytics", "true"]) != 0
+    out = caplog.text
+    assert "Not inside a DVC repo" in out
+
+
 @pytest.mark.parametrize(
     "args, ret, msg",
     [
@@ -128,6 +134,21 @@ def test_config_get(tmp_dir, dvc, capsys, caplog, args, ret, msg):
     assert msg in text
 
 
+@pytest.mark.parametrize(
+    "args, ret",
+    [
+        (["--local", "core.remote"], 251),
+        (["--project", "core.remote"], 251),
+        (["core.remote"], 0),
+    ],
+)
+def test_config_get_in_non_dvc_repo(tmp_dir, caplog, args, ret):
+    assert main(["config"] + args) == ret
+    if ret != 0:
+        out = caplog.text
+        assert "Not inside a DVC repo" in out
+
+
 def test_config_list(tmp_dir, dvc, capsys):
     (tmp_dir / ".dvc" / "config").write_text(
         textwrap.dedent(
@@ -169,6 +190,21 @@ def test_config_list(tmp_dir, dvc, capsys):
     assert "core.remote=myremote" in out
 
 
+@pytest.mark.parametrize(
+    "args, ret",
+    [
+        (["--list", "--local"], 251),
+        (["--list", "--project"], 251),
+        (["--list"], 0),
+    ],
+)
+def test_config_list_in_non_dvc_repo(tmp_dir, caplog, args, ret):
+    assert main(["config"] + args) == ret
+    if ret != 0:
+        out = caplog.text
+        assert "Not inside a DVC repo" in out
+
+
 @pytest.mark.parametrize(
     "args", [["core.analytics"], ["core.analytics", "false"], ["--unset"]]
 )