Repository: AnalogJ/lexicon

Issue:
[CLI] Pretty output for list method
Is there any plans to have pretty outputs (table or at least formatted) for the ```list``` operation on the CLI?

Right now, the CLI assumes a verbosity of DEBUG level, and outputs the Python representation of the result (managed by the provider). If --log_level=ERROR is used, no output is generated, which defeats the CLI usage, in my opinion.

Is this behavior expected? Would you be open to a PR for that?

Discussion:

Patch:
diff --git a/lexicon/__main__.py b/lexicon/__main__.py
index d674809e..ad243f18 100644
--- a/lexicon/__main__.py
+++ b/lexicon/__main__.py
@@ -7,6 +7,7 @@ import importlib
 import logging
 import os
 import sys
+import json
 
 import pkg_resources
 
@@ -19,16 +20,19 @@ logger = logging.getLogger(__name__)
 
 def BaseProviderParser():
     parser = argparse.ArgumentParser(add_help=False)
-    parser.add_argument("action", help="specify the action to take", default='list', choices=['create', 'list', 'update', 'delete'])
-    parser.add_argument("domain", help="specify the domain, supports subdomains as well")
-    parser.add_argument("type", help="specify the entry type", default='TXT', choices=['A', 'AAAA', 'CNAME', 'MX', 'NS', 'SOA', 'TXT', 'SRV', 'LOC'])
-
-    parser.add_argument("--name", help="specify the record name")
-    parser.add_argument("--content", help="specify the record content")
-    parser.add_argument("--ttl", type=int, help="specify the record time-to-live")
-    parser.add_argument("--priority", help="specify the record priority")
-    parser.add_argument("--identifier", help="specify the record for update or delete actions")
-    parser.add_argument("--log_level", help="specify the log level", default="DEBUG", choices=["CRITICAL","ERROR","WARNING","INFO","DEBUG","NOTSET"])
+    parser.add_argument('action', help='specify the action to take', default='list', choices=['create', 'list', 'update', 'delete'])
+    parser.add_argument('domain', help='specify the domain, supports subdomains as well')
+    parser.add_argument('type', help='specify the entry type', default='TXT', choices=['A', 'AAAA', 'CNAME', 'MX', 'NS', 'SOA', 'TXT', 'SRV', 'LOC'])
+
+    parser.add_argument('--name', help='specify the record name')
+    parser.add_argument('--content', help='specify the record content')
+    parser.add_argument('--ttl', type=int, help='specify the record time-to-live')
+    parser.add_argument('--priority', help='specify the record priority')
+    parser.add_argument('--identifier', help='specify the record for update or delete actions')
+    parser.add_argument('--log_level', help='specify the log level', default='ERROR', choices=['CRITICAL','ERROR','WARNING','INFO','DEBUG','NOTSET'])
+    parser.add_argument('--output', 
+                        help='specify the type of output: by default a formatted table (TABLE), a formatted table without header (TABLE-NO-HEADER), a JSON string (JSON) or no output (QUIET)',
+                        default='TABLE', choices=['TABLE', 'TABLE-NO-HEADER', 'JSON', 'QUIET'])
     return parser
 
 def MainParser():
@@ -43,11 +47,11 @@ def MainParser():
 
     parser = argparse.ArgumentParser(description='Create, Update, Delete, List DNS entries')
     try:
-        version = pkg_resources.get_distribution("dns-lexicon").version
+        version = pkg_resources.get_distribution('dns-lexicon').version
     except pkg_resources.DistributionNotFound:
         version = 'unknown'
-    parser.add_argument('--version', help="show the current version of lexicon", action='version', version='%(prog)s {0}'.format(version))
-    parser.add_argument('--delegated', help="specify the delegated domain")
+    parser.add_argument('--version', help='show the current version of lexicon', action='version', version='%(prog)s {0}'.format(version))
+    parser.add_argument('--delegated', help='specify the delegated domain')
     subparsers = parser.add_subparsers(dest='provider_name', help='specify the DNS provider to use')
     subparsers.required = True
 
@@ -60,17 +64,73 @@ def MainParser():
 
     return parser
 
-#dynamically determine all the providers available.
+# Convert returned JSON into a nice table for command line usage
+def generate_table_result(logger, output=None, without_header=None):
+    try:
+        _ = (entry for entry in output)
+    except:
+        logger.debug('Command output is not iterable, and then cannot be printed with --quiet parameter not enabled.')
+        return None
+
+    array = [[row['id'], row['type'], row['name'], row['content'], row['ttl']] for row in output]
+
+    # Insert header (insert before calculating the max width of each column to take headers size into account)
+    if not without_header:
+        headers = ['ID', 'TYPE', 'NAME', 'CONTENT', 'TTL']
+        array.insert(0, headers)
+
+    columnWidths = [0, 0, 0, 0, 0]
+    # Find max width for each column
+    for row in array:
+        for idx, col in enumerate(row):
+            width = len(str(col))
+            if width > columnWidths[idx]:
+                columnWidths[idx] = width
+
+    # Add a 'nice' separator
+    if not without_header:
+        array.insert(1, ['-' * columnWidths[idx] for idx in range(len(columnWidths))])
+
+    # Construct table to be printed
+    table = []
+    for row in array:
+        rowList = []
+        for idx, col in enumerate(row):
+            rowList.append(str(col).ljust(columnWidths[idx]))
+        table.append(' '.join(rowList))
+
+    # Return table
+    return '\n'.join(table)
+
+# Print the relevant output for given output_type
+def handle_output(results, output_type):
+    if not output_type == 'QUIET':
+        if not output_type == 'JSON':
+            table = generate_table_result(logger, results, output_type == 'TABLE-NO-HEADER')
+            if table:
+                print(table)
+        else:
+            try:
+                _ = (entry for entry in results)
+                json_str = json.dumps(results)
+                if json_str:
+                    print(json_str)
+            except:
+                logger.debug('Output is not a JSON, and then cannot be printed with --output=JSON parameter.')
+                pass
+
+# Dynamically determine all the providers available.
 def main():
-
     parsed_args = MainParser().parse_args()
     log_level = logging.getLevelName(parsed_args.log_level)
     logging.basicConfig(stream=sys.stdout, level=log_level, format='%(message)s')
 
     logger.debug('Arguments: %s', parsed_args)
-    client = Client(parsed_args.__dict__)
-    client.execute()
+    client = Client(vars(parsed_args))
+    
+    results = client.execute()
 
+    handle_output(results, parsed_args.output)
 
 if __name__ == '__main__':
     main()


Test patch:
diff --git a/tests/test_client.py b/tests/test_client.py
index 68f5b1f7..d41fc1e7 100644
--- a/tests/test_client.py
+++ b/tests/test_client.py
@@ -1,6 +1,7 @@
 import lexicon.client
 import pytest
 import os
+
 def test_Client_init():
     options = {
         'provider_name':'base',
diff --git a/tests/test_output.py b/tests/test_output.py
new file mode 100644
index 00000000..fdfae206
--- /dev/null
+++ b/tests/test_output.py
@@ -0,0 +1,67 @@
+from __future__ import absolute_import
+
+import sys
+import importlib
+import logging
+import json
+import lexicon.__main__
+
+from types import ModuleType
+from lexicon.providers.base import Provider as BaseProvider
+
+logger = logging.getLogger(__name__)
+data = [
+    {'id': 'fake-id', 'type': 'TXT', 'name': 'fake.example.com', 'content': 'fake', 'ttl': 3600},
+    {'id': 'fake2-id', 'type': 'TXT', 'name': 'fake2.example.com', 'content': 'fake2', 'ttl': 3600}
+]
+
+# Ensure that stdout corresponds to the given reference output
+def assert_correct_output(capsys, expected_output_lines):
+    out, _ = capsys.readouterr()
+    assert out.splitlines() == expected_output_lines
+
+def test_output_function_outputs_json_as_table(capsys):
+    expected_output_lines = [
+        'ID       TYPE NAME              CONTENT TTL ',
+        '-------- ---- ----------------- ------- ----',
+        'fake-id  TXT  fake.example.com  fake    3600',
+        'fake2-id TXT  fake2.example.com fake2   3600',
+    ]
+
+    lexicon.__main__.handle_output(data, 'TABLE')
+    assert_correct_output(capsys, expected_output_lines)
+
+def test_output_function_outputs_json_as_table_with_no_header(capsys):
+    expected_output_lines = [
+        'fake-id  TXT fake.example.com  fake  3600',
+        'fake2-id TXT fake2.example.com fake2 3600',
+    ]
+
+    lexicon.__main__.handle_output(data, 'TABLE-NO-HEADER')
+    assert_correct_output(capsys, expected_output_lines)
+
+def test_output_function_outputs_json_as_json_string(capsys):
+    lexicon.__main__.handle_output(data, 'JSON')
+
+    out, _ = capsys.readouterr()
+    json_data = json.loads(out)
+
+    assert json_data == data
+
+def test_output_function_output_nothing_when_quiet(capsys):
+    expected_output_lines = []
+
+    lexicon.__main__.handle_output(data, 'QUIET')
+    assert_correct_output(capsys, expected_output_lines)
+
+def test_output_function_outputs_nothing_with_not_a_json_data(capsys):
+    expected_output_lines = []
+
+    lexicon.__main__.handle_output(True, 'TABLE')
+    assert_correct_output(capsys, expected_output_lines)
+
+    lexicon.__main__.handle_output(True, 'TABLE-NO-HEADER')
+    assert_correct_output(capsys, expected_output_lines)
+
+    lexicon.__main__.handle_output(True, 'JSON')
+    assert_correct_output(capsys, expected_output_lines)
\ No newline at end of file
diff --git a/tests/test_main.py b/tests/test_parser.py
similarity index 94%
rename from tests/test_main.py
rename to tests/test_parser.py
index bbfd0d3d..20cd7cac 100644
--- a/tests/test_main.py
+++ b/tests/test_parser.py
@@ -8,7 +8,7 @@ def test_BaseProviderParser():
     assert parsed.domain == 'capsulecd.com'
     assert parsed.type == 'TXT'
     assert parsed.ttl == None
-
+    assert parsed.output == 'TABLE'
 
 def test_BaseProviderParser_without_domain():
     baseparser = lexicon.__main__.BaseProviderParser()
@@ -27,6 +27,7 @@ def test_MainParser():
     assert parsed.action == 'list'
     assert parsed.domain == 'capsulecd.com'
     assert parsed.type == 'TXT'
+    assert parsed.output == 'TABLE'
 
 def test_MainParser_without_args():
     baseparser = lexicon.__main__.MainParser()