Machine: use table for list outputs(#6892)

fix: #6892

Current output of machine list is quite urgly, it is originaly used to
show configurations. In this PR, we use a new table format for it.

dvc/command/machine.py

@@ -2,6 +2,7 @@ import argparse
 
 from dvc.command.base import CmdBase, append_doc_link, fix_subparsers
 from dvc.command.config import CmdConfig
+from dvc.compare import TabularData
 from dvc.config import ConfigError
 from dvc.exceptions import DvcException
 from dvc.ui import ui
@@ -71,18 +72,56 @@ class CmdMachineRemove(CmdMachineConfig):
 
 
 class CmdMachineList(CmdMachineConfig):
-    def run(self):
+    TABLE_COLUMNS = [
+        "name",
+        "cloud",
+        "region",
+        "image",
+        "spot",
+        "spot_price",
+        "instance_hdd_size",
+        "instance_type",
+        "ssh_private",
+        "startup_script",
+    ]
+
+    PRIVATE_COLUMNS = ["ssh_private", "startup_script"]
+
+    def _hide_private(self, conf):
+        for machine in conf:
+            for column in self.PRIVATE_COLUMNS:
+                if column in conf[machine]:
+                    conf[machine][column] = "***"
+
+    def _show_origin(self):
         levels = [self.args.level] if self.args.level else self.config.LEVELS
         for level in levels:
             conf = self.config.read(level)["machine"]
             if self.args.name:
                 conf = conf.get(self.args.name, {})
-            prefix = self._config_file_prefix(
-                self.args.show_origin, self.config, level
-            )
+            self._hide_private(conf)
+            prefix = self._config_file_prefix(True, self.config, level)
             configs = list(self._format_config(conf, prefix))
             if configs:
                 ui.write("\n".join(configs))
+
+    def _show_table(self):
+        td = TabularData(self.TABLE_COLUMNS, fill_value="-")
+        conf = self.config.read()["machine"]
+        if self.args.name:
+            conf = {self.args.name: conf.get(self.args.name, {})}
+        self._hide_private(conf)
+        for machine, machine_config in conf.items():
+            machine_config["name"] = machine
+            td.row_from_dict(machine_config)
+        td.dropna("cols", "all")
+        td.render()
+
+    def run(self):
+        if self.args.show_origin:
+            self._show_origin()
+        else:
+            self._show_table()
         return 0
 
 

dvc/compare.py

@@ -188,9 +188,6 @@ class TabularData(MutableSequence[Sequence["CellT"]]):
                 f"Invalid 'axis' value {axis}."
                 "Choose one of ['rows', 'cols']"
             )
-<<<<<<< HEAD
-        to_drop: Set = set()
-=======
         if how not in ["any", "all"]:
             raise ValueError(
                 f"Invalid 'how' value {how}." "Choose one of ['any', 'all']"
@@ -201,7 +198,6 @@ class TabularData(MutableSequence[Sequence["CellT"]]):
         if how == "all":
             match = False
 
->>>>>>> 8ad4b8aa (TabularData: add a new how `arg` to dropna(#6892))
         for n_row, row in enumerate(self):
             for n_col, col in enumerate(row):
                 if (col == self._fill_value) is match:

tests/func/experiments/test_show.py

@@ -13,6 +13,7 @@ from dvc.repo.experiments.utils import exp_refs_by_rev
 from dvc.utils.fs import makedirs
 from dvc.utils.serialize import YAMLFileCorruptedError
 from tests.func.test_repro_multistage import COPY_SCRIPT
+from tests.utils import console_width
 
 
 def test_show_simple(tmp_dir, scm, dvc, exp_stage):
@@ -270,8 +271,6 @@ def test_show_filter(
     included,
     excluded,
 ):
-    from contextlib import contextmanager
-
     from dvc.ui import ui
 
     capsys.readouterr()
@@ -317,21 +316,7 @@ def test_show_filter(
     if e_params is not None:
         command.append(f"--exclude-params={e_params}")
 
-    @contextmanager
-    def console_with(console, width):
-        console_options = console.options
-        original = console_options.max_width
-        con_width = console._width
-
-        try:
-            console_options.max_width = width
-            console._width = width
-            yield
-        finally:
-            console_options.max_width = original
-            console._width = con_width
-
-    with console_with(ui.rich_console, 255):
+    with console_width(ui.rich_console, 255):
         assert main(command) == 0
     cap = capsys.readouterr()
 

tests/func/machine/test_machine_config.py

@@ -5,6 +5,8 @@ import pytest
 import tpi
 
 from dvc.main import main
+from dvc.ui import ui
+from tests.utils import console_width
 
 from .conftest import BASIC_CONFIG
 
@@ -88,31 +90,26 @@ FULL_CONFIG_TEXT = textwrap.dedent(
 
 
 def test_machine_list(tmp_dir, dvc, capsys):
+    from dvc.command.machine import CmdMachineList
+
     (tmp_dir / ".dvc" / "config").write_text(FULL_CONFIG_TEXT)
 
-    assert main(["machine", "list"]) == 0
-    cap = capsys.readouterr()
-    assert "cloud=azure" in cap.out
+    with console_width(ui.rich_console, 255):
+        assert main(["machine", "list"]) == 0
+    out, _ = capsys.readouterr()
+    for key in CmdMachineList.TABLE_COLUMNS:
+        assert f"{key}" in out
+    assert "bar     azure    -         -" in out
+    assert "foo     aws      us-west   iterative-cml  True    1.2345" in out
+    assert "10                   l                ***            ***" in out
+    assert "tesla" in out
 
-    assert main(["machine", "list", "foo"]) == 0
-    cap = capsys.readouterr()
-    assert "cloud=azure" not in cap.out
-    assert "cloud=aws" in cap.out
-    assert "region=us-west" in cap.out
-    assert "image=iterative-cml" in cap.out
-    assert "name=iterative_test" in cap.out
-    assert "spot=True" in cap.out
-    assert "spot_price=1.2345" in cap.out
-    assert "instance_hdd_size=10" in cap.out
-    assert "instance_type=l" in cap.out
-    assert "instance_gpu=tesla" in cap.out
-    assert "ssh_private=secret" in cap.out
-    assert (
-        "startup_script={}".format(
-            os.path.join(tmp_dir, ".dvc", "..", "start.sh")
-        )
-        in cap.out
-    )
+    with console_width(ui.rich_console, 255):
+        assert main(["machine", "list", "bar"]) == 0
+    out, _ = capsys.readouterr()
+    assert "foo" not in out
+    assert "name    cloud" in out
+    assert "bar     azure" in out
 
 
 def test_machine_rename_success(

tests/unit/command/test_machine.py

@@ -1,3 +1,5 @@
+import os
+
 import configobj
 import pytest
 from mock import call
@@ -110,16 +112,24 @@ def test_ssh(tmp_dir, dvc, mocker):
     m.assert_called_once_with("foo")
 
 
-def test_list(tmp_dir, mocker):
+@pytest.mark.parametrize("show_origin", [["--show-origin"], []])
+def test_list(tmp_dir, mocker, show_origin):
+    from dvc.compare import TabularData
     from dvc.ui import ui
 
     tmp_dir.gen(DATA)
-    cli_args = parse_args(["machine", "list", "foo"])
+    cli_args = parse_args(["machine", "list"] + show_origin + ["foo"])
     assert cli_args.func == CmdMachineList
     cmd = cli_args.func(cli_args)
-    m = mocker.patch.object(ui, "write", autospec=True)
+    if show_origin:
+        m = mocker.patch.object(ui, "write", autospec=True)
+    else:
+        m = mocker.patch.object(TabularData, "render", autospec=True)
     assert cmd.run() == 0
-    m.assert_called_once_with("cloud=aws")
+    if show_origin:
+        m.assert_called_once_with(f".dvc{os.sep}config	cloud=aws")
+    else:
+        m.assert_called_once()
 
 
 def test_modified(tmp_dir):

tests/utils/__init__.py

@@ -54,3 +54,18 @@ def clean_staging():
         )
     except FileNotFoundError:
         pass
+
+
+@contextmanager
+def console_width(console, width):
+    console_options = console.options
+    original = console_options.max_width
+    con_width = console._width
+
+    try:
+        console_options.max_width = width
+        console._width = width
+        yield
+    finally:
+        console_options.max_width = original
+        console._width = con_width