From 9e43f12337c14bd67ed961145f796fdc69126d56 Mon Sep 17 00:00:00 2001 From: Rick Bartra Date: Wed, 10 Oct 2018 15:30:54 -0400 Subject: [PATCH] Pegleg CLI output improvement This commit leverages python prettytable to create tables and output CLI information for the following: - site: - lint - list - show - type: - list - repo: - lint Addtionally, this commit changes the verbosity settings for pegleg CLI. When verbosity is not set, only error logs will be shown as this would be useful to users when errors do occur. Otherwise, no logs should be shown in the CLI output unless the user passes the `verbose` flag. Change-Id: Ic7782e9e383a1d6a7e31ff7cce025beb53c7db01 --- pegleg/cli.py | 2 +- pegleg/engine/lint.py | 20 ++++++++++-- pegleg/engine/site.py | 31 +++++++++++++------ pegleg/engine/type.py | 15 ++++----- tests/unit/test_cli.py | 70 ++++++++++++++++-------------------------- 5 files changed, 75 insertions(+), 63 deletions(-) diff --git a/pegleg/cli.py b/pegleg/cli.py index 74f9a56c..9035794e 100644 --- a/pegleg/cli.py +++ b/pegleg/cli.py @@ -110,7 +110,7 @@ def main(*, verbose): if verbose: log_level = logging.DEBUG else: - log_level = logging.INFO + log_level = logging.ERROR logging.basicConfig(format=LOG_FORMAT, level=log_level) diff --git a/pegleg/engine/lint.py b/pegleg/engine/lint.py index c79919ba..100aa86d 100644 --- a/pegleg/engine/lint.py +++ b/pegleg/engine/lint.py @@ -16,8 +16,12 @@ import click import logging import os import pkg_resources +import shutil +import textwrap import yaml +from prettytable import PrettyTable + from pegleg import config from pegleg.engine.errorcodes import DOCUMENT_LAYER_MISMATCH from pegleg.engine.errorcodes import FILE_CONTAINS_INVALID_YAML @@ -150,15 +154,27 @@ def _filter_messages_by_warn_and_error_lint(*, errors = [] warns = [] + # Create tables to output CLI results + errors_table = PrettyTable() + errors_table.field_names = ['error_code', 'error_message'] + warnings_table = PrettyTable() + warnings_table.field_names = ['warning_code', 'warning_message'] + # Calculate terminal size to always make sure that the table output + # is readable regardless of screen size + line_length = int(shutil.get_terminal_size().columns / 1.5) for code, message in messages: if code in warn_lint: warns.append('%s: %s' % (code, message)) + warnings_table.add_row([code, textwrap.fill(message, line_length)]) elif code not in exclude_lint: errors.append('%s: %s' % (code, message)) + errors_table.add_row([code, textwrap.fill(message, line_length)]) if errors: - raise click.ClickException('\n'.join( - ['Linting failed:'] + errors + ['Linting warnings:'] + warns)) + raise click.ClickException('Linting failed:\n' + + errors_table.get_string() + + '\nLinting warnings:\n' + + warnings_table.get_string()) return warns diff --git a/pegleg/engine/site.py b/pegleg/engine/site.py index 89b8de5e..8846e892 100644 --- a/pegleg/engine/site.py +++ b/pegleg/engine/site.py @@ -12,14 +12,14 @@ # See the License for the specific language governing permissions and # limitations under the License. -import csv -import json import logging import os import click import yaml +from prettytable import PrettyTable + from pegleg.engine import util __all__ = ('collect', 'list_', 'show', 'render') @@ -115,11 +115,10 @@ def render(site_name, output_stream): def list_(output_stream): """List site names for a given repository.""" - # TODO(felipemonteiro): This should output a formatted table, not rows of - # data without delimited columns. - fieldnames = ['site_name', 'site_type', 'repositories'] - writer = csv.DictWriter( - output_stream, fieldnames=fieldnames, delimiter=' ') + # Create a table to output site information for all sites for a given repo + site_table = PrettyTable() + site_table.field_names = ['site_name', 'site_type'] + for site_name in util.files.list_sites(): params = util.definition.load_as_params(site_name) # TODO(felipemonteiro): This is a temporary hack around legacy manifest @@ -131,10 +130,24 @@ def list_(output_stream): # a configuration via a "set_site_revision" function, for example. if 'revision' in params: params.pop('revision') - writer.writerow(params) + site_table.add_row([params['site_name'], params['site_type']]) + # Write table to specified output_stream + output_stream.write(site_table.get_string() + "\n") def show(site_name, output_stream): data = util.definition.load_as_params(site_name) data['files'] = list(util.definition.site_files(site_name)) - json.dump(data, output_stream, indent=2, sort_keys=True) + # Create a table to output site information for specific site + site_table = PrettyTable() + site_table.field_names = ['revision', 'site_name', 'site_type', 'files'] + if 'revision' in data.keys(): + for file in data['files']: + site_table.add_row([data['revision'], data['site_name'], + data['site_type'], file]) + else: + for file in data['files']: + site_table.add_row(["", data['site_name'], + data['site_type'], file]) + # Write tables to specified output_stream + output_stream.write(site_table.get_string() + "\n") diff --git a/pegleg/engine/type.py b/pegleg/engine/type.py index 6661d2b2..5405ad56 100644 --- a/pegleg/engine/type.py +++ b/pegleg/engine/type.py @@ -12,9 +12,10 @@ # See the License for the specific language governing permissions and # limitations under the License. -import csv import logging +from prettytable import PrettyTable + from pegleg.engine import util __all__ = ('list_types', ) @@ -25,10 +26,10 @@ LOG = logging.getLogger(__name__) def list_types(output_stream): """List type names for a given repository.""" - # TODO(felipemonteiro): This should output a formatted table, not rows of - # data without delimited columns. - fieldnames = ['type_name'] - writer = csv.DictWriter( - output_stream, fieldnames=fieldnames, delimiter=' ') + # Create a table to output site types for a given repo + type_table = PrettyTable() + type_table.field_names = ['type_name'] for type_name in util.files.list_types(): - writer.writerow({'type_name': type_name}) + type_table.add_row([type_name]) + # Write table to specified output_stream + output_stream.write(type_table.get_string() + "\n") diff --git a/tests/unit/test_cli.py b/tests/unit/test_cli.py index 8048c6fc..a3169887 100644 --- a/tests/unit/test_cli.py +++ b/tests/unit/test_cli.py @@ -211,17 +211,13 @@ class TestSiteCliActions(BaseCLIActionTest): repo_url = 'https://github.com/openstack/%s@%s' % (self.repo_name, self.repo_rev) - # NOTE(felipemonteiro): Pegleg currently doesn't dump a table to stdout - # for this CLI call so mock out the csv DictWriter to determine output. - with mock.patch('pegleg.engine.site.csv.DictWriter') as mock_writer: + # Mock out PrettyTable to determine output. + with mock.patch('pegleg.engine.site.PrettyTable') as mock_writer: result = self.runner.invoke(cli.site, ['-r', repo_url, 'list']) - assert result.exit_code == 0 m_writer = mock_writer.return_value - m_writer.writerow.assert_called_once_with({ - 'site_type': 'foundry', - 'site_name': self.site_name - }) + m_writer.add_row.assert_called_with([self.site_name, + 'foundry']) def test_list_sites_using_local_path(self): """Validates list action using local repo path.""" @@ -231,17 +227,13 @@ class TestSiteCliActions(BaseCLIActionTest): repo_path = self.treasuremap_path - # NOTE(felipemonteiro): Pegleg currently doesn't dump a table to stdout - # for this CLI call so mock out the csv DictWriter to determine output. - with mock.patch('pegleg.engine.site.csv.DictWriter') as mock_writer: + # Mock out PrettyTable to determine output. + with mock.patch('pegleg.engine.site.PrettyTable') as mock_writer: result = self.runner.invoke(cli.site, ['-r', repo_path, 'list']) - assert result.exit_code == 0 m_writer = mock_writer.return_value - m_writer.writerow.assert_called_once_with({ - 'site_type': 'foundry', - 'site_name': self.site_name - }) + m_writer.add_row.assert_called_with([self.site_name, + 'foundry']) ### Show tests ### @@ -254,18 +246,15 @@ class TestSiteCliActions(BaseCLIActionTest): repo_url = 'https://github.com/openstack/%s@%s' % (self.repo_name, self.repo_rev) - with mock.patch('pegleg.engine.site.json') as mock_json: + with mock.patch('pegleg.engine.site.PrettyTable') as mock_writer: result = self.runner.invoke( cli.site, ['-r', repo_url, 'show', self.site_name]) - assert result.exit_code == 0 - assert mock_json.dump.called - mock_calls = mock_json.dump.mock_calls - assert mock_calls[0][1][0] == { - 'files': mock.ANY, - 'site_type': 'foundry', - 'site_name': self.site_name - } + m_writer = mock_writer.return_value + m_writer.add_row.assert_called_with(['', + self.site_name, + 'foundry', + mock.ANY]) def test_show_site_using_local_path(self): """Validates show action using local repo path.""" @@ -274,18 +263,15 @@ class TestSiteCliActions(BaseCLIActionTest): # 1) Show site (should skip clone repo) repo_path = self.treasuremap_path - with mock.patch('pegleg.engine.site.json') as mock_json: + with mock.patch('pegleg.engine.site.PrettyTable') as mock_writer: result = self.runner.invoke( cli.site, ['-r', repo_path, 'show', self.site_name]) - assert result.exit_code == 0 - assert mock_json.dump.called - mock_calls = mock_json.dump.mock_calls - assert mock_calls[0][1][0] == { - 'files': mock.ANY, - 'site_type': 'foundry', - 'site_name': self.site_name - } + m_writer = mock_writer.return_value + m_writer.add_row.assert_called_with(['', + self.site_name, + 'foundry', + mock.ANY]) ### Render tests ### @@ -398,14 +384,12 @@ class TestTypeCliActions(BaseCLIActionTest): repo_url = 'https://github.com/openstack/%s@%s' % (self.repo_name, self.repo_rev) - # NOTE(felipemonteiro): Pegleg currently doesn't dump a table to stdout - # for this CLI call so mock out the csv DictWriter to determine output. - with mock.patch('pegleg.engine.type.csv.DictWriter') as mock_writer: + # Mock out PrettyTable to determine output. + with mock.patch('pegleg.engine.type.PrettyTable') as mock_writer: result = self.runner.invoke(cli.type, ['-r', repo_url, 'list']) - assert result.exit_code == 0 m_writer = mock_writer.return_value - m_writer.writerow.assert_any_call({'type_name': 'foundry'}) + m_writer.add_row.assert_called_with(['foundry']) def test_list_types_using_local_repo_path(self): """Validates list types action using local repo path.""" @@ -415,11 +399,9 @@ class TestTypeCliActions(BaseCLIActionTest): repo_path = self.treasuremap_path - # NOTE(felipemonteiro): Pegleg currently doesn't dump a table to stdout - # for this CLI call so mock out the csv DictWriter to determine output. - with mock.patch('pegleg.engine.type.csv.DictWriter') as mock_writer: + # Mock out PrettyTable to determine output. + with mock.patch('pegleg.engine.type.PrettyTable') as mock_writer: result = self.runner.invoke(cli.type, ['-r', repo_path, 'list']) - assert result.exit_code == 0 m_writer = mock_writer.return_value - m_writer.writerow.assert_any_call({'type_name': 'foundry'}) + m_writer.add_row.assert_called_with(['foundry'])