From 3cc5ea73acbccfaa778638ef8f87f6fd2f55bb59 Mon Sep 17 00:00:00 2001 From: Jiashuo Li Date: Tue, 10 Mar 2020 18:25:26 +0800 Subject: [PATCH 1/2] Support --only-show-errors --- docs/logging.md | 1 + examples/exapp2 | 81 +++++++++++++++++++++++++++++++++++++-- knack/cli.py | 1 + knack/invocation.py | 15 ++++---- knack/log.py | 42 ++++++++++++++------ tests/test_deprecation.py | 4 +- tests/test_help.py | 41 +++++++++++--------- tests/test_log.py | 48 ++++++++++++++--------- tests/test_preview.py | 32 ++++++++++++++++ 9 files changed, 204 insertions(+), 61 deletions(-) diff --git a/docs/logging.md b/docs/logging.md index 1ce1ccf..aea975e 100644 --- a/docs/logging.md +++ b/docs/logging.md @@ -12,6 +12,7 @@ Logging - By default, log messages Warning and above are shown to the user. - `--verbose` - This flag changes the logging level to Info and above. - `--debug` - This flag changes the logging level to Debug and above. +- `--only-show-errors` - This flag changes the logging level to Error only, suppressing Warning. * All log messages go to STDERR (not STDOUT) * Log to Error or Warning for user messages instead of using the `print()` function diff --git a/examples/exapp2 b/examples/exapp2 index 154d63d..49a59e5 100644 --- a/examples/exapp2 +++ b/examples/exapp2 @@ -28,6 +28,22 @@ helps['abc list'] = """ text: {cli_name} abc list """.format(cli_name=cli_name) +helps['abc first'] = """ + type: command + short-summary: List the first several letters in the alphabet. + examples: + - name: Show the list of abc + text: {cli_name} abc first --number 3 +""".format(cli_name=cli_name) + +helps['abc last'] = """ + type: command + short-summary: List the last several letters in the alphabet. + examples: + - name: Show the list of xyz + text: {cli_name} abc last --number 3 +""".format(cli_name=cli_name) + def a_test_command_handler(): return [{'a': 1, 'b': 1234}, {'a': 3, 'b': 4}] @@ -38,9 +54,55 @@ def abc_list_command_handler(): return list(string.ascii_lowercase) +def abc_first_command_handler(number=5): + import string + return list(string.ascii_lowercase)[0:number] + + +def abc_last_command_handler(number=5): + import string + return list(string.ascii_lowercase)[-number:] + + +def num_range_command_handler(start=0, end=5): + """ + Get a list of natural numbers from start to end + :param start: the lower bound + :param end: the higher bound + :return: + """ + return list(range(int(start), int(end) + 1)) + + +def sample_json_handler(): + """ + Get a sample JSON dict + """ + # https://docs.microsoft.com/en-us/rest/api/resources/subscriptions/list#examples + result = { + "id": "/subscriptions/291bba3f-e0a5-47bc-a099-3bdcb2a50a05", + "subscriptionId": "291bba3f-e0a5-47bc-a099-3bdcb2a50a05", + "tenantId": "31c75423-32d6-4322-88b7-c478bdde4858", + "displayName": "Example Subscription", + "state": "Enabled", + "subscriptionPolicies": { + "locationPlacementId": "Internal_2014-09-01", + "quotaId": "Internal_2014-09-01", + "spendingLimit": "Off" + }, + "authorizationSource": "RoleBased", + "managedByTenants": [ + { + "tenantId": "8f70baf1-1f6e-46a2-a1ff-238dac1ebfb7" + } + ] + } + return result + def hello_command_handler(myarg=None, abc=None): return ['hello', 'world', myarg, abc] + WELCOME_MESSAGE = r""" _____ _ _____ / ____| | |_ _| @@ -67,15 +129,28 @@ class MyCommandsLoader(CLICommandsLoader): def load_command_table(self, args): with CommandGroup(self, 'hello', '__main__#{}') as g: g.command('world', 'hello_command_handler', confirmation=True) + with CommandGroup(self, '', '__main__#{}') as g: + g.command('sample-json', 'sample_json_handler') with CommandGroup(self, 'abc', '__main__#{}') as g: - g.command('list', 'abc_list_command_handler') - g.command('show', 'a_test_command_handler') - g.command('get', 'a_test_command_handler', deprecate_info=g.deprecate(redirect='show', hide='0.1.0')) + g.command('list', 'abc_list_command_handler') + g.command('show', 'a_test_command_handler') + g.command('get', 'a_test_command_handler', deprecate_info=g.deprecate(redirect='show', hide='1.0.0')) + g.command('first', 'abc_first_command_handler', is_preview=True) + g.command('last', 'abc_last_command_handler', ) + with CommandGroup(self, 'ga', '__main__#{}') as g: + g.command('range', 'num_range_command_handler') + with CommandGroup(self, 'pre', '__main__#{}', is_preview=True) as g: + g.command('range', 'num_range_command_handler') + with CommandGroup(self, 'exp', '__main__#{}', ) as g: + g.command('range', 'num_range_command_handler') return super(MyCommandsLoader, self).load_command_table(args) def load_arguments(self, command): with ArgumentsContext(self, 'hello world') as ac: ac.argument('myarg', type=int, default=100) + with ArgumentsContext(self, 'ga range') as ac: + ac.argument('start', type=int, is_preview=True) + ac.argument('end', type=int, ) super(MyCommandsLoader, self).load_arguments(command) diff --git a/knack/cli.py b/knack/cli.py index 7b54112..dbf3a25 100644 --- a/knack/cli.py +++ b/knack/cli.py @@ -91,6 +91,7 @@ def __init__(self, self.output = self.output_cls(cli_ctx=self) self.result = None self.query = query_cls(cli_ctx=self) + self.only_show_errors = False @staticmethod def _should_show_version(args): diff --git a/knack/invocation.py b/knack/invocation.py index b217f90..7026afb 100644 --- a/knack/invocation.py +++ b/knack/invocation.py @@ -139,7 +139,7 @@ def execute(self, args): self.parser.load_command_table(self.commands_loader) self.cli_ctx.raise_event(EVENT_INVOKER_CMD_TBL_LOADED, parser=self.parser) - arg_check = [a for a in args if a not in ['--verbose', '--debug']] + arg_check = [a for a in args if a not in ['--verbose', '--debug', '--only-show-warnings']] if not arg_check: self.cli_ctx.completion.enable_autocomplete(self.parser) subparser = self.parser.subparsers[tuple()] @@ -198,12 +198,13 @@ def execute(self, args): preview_kwargs['object_type'] = 'command' previews.append(ImplicitPreviewItem(**preview_kwargs)) - colorama.init() - for d in deprecations: - print(d.message, file=sys.stderr) - for p in previews: - print(p.message, file=sys.stderr) - colorama.deinit() + if not self.cli_ctx.only_show_errors: + colorama.init() + for d in deprecations: + print(d.message, file=sys.stderr) + for p in previews: + print(p.message, file=sys.stderr) + colorama.deinit() cmd_result = parsed_args.func(params) cmd_result = todict(cmd_result) diff --git a/knack/log.py b/knack/log.py index 8f37300..9f5d3ac 100644 --- a/knack/log.py +++ b/knack/log.py @@ -7,7 +7,7 @@ import logging from logging.handlers import RotatingFileHandler -from .util import CtxTypeError, ensure_dir +from .util import CtxTypeError, ensure_dir, CLIError from .events import EVENT_PARSER_GLOBAL_CREATE CLI_LOGGER_NAME = 'cli' @@ -88,6 +88,7 @@ class CLILogging(object): DEBUG_FLAG = '--debug' VERBOSE_FLAG = '--verbose' + ONLY_SHOW_ERRORS_FLAG = '--only-show-errors' @staticmethod def on_global_arguments(_, **kwargs): @@ -97,6 +98,9 @@ def on_global_arguments(_, **kwargs): help='Increase logging verbosity. Use --debug for full debug logs.') arg_group.add_argument(CLILogging.DEBUG_FLAG, dest='_log_verbosity_debug', action='store_true', help='Increase logging verbosity to show all debug logs.') + arg_group.add_argument(CLILogging.ONLY_SHOW_ERRORS_FLAG, dest='_log_verbosity_only_show_errors', + action='store_true', + help='Only show errors, suppressing warnings.') def __init__(self, name, cli_ctx=None): """ @@ -123,8 +127,8 @@ def configure(self, args): :param args: The arguments from the command line :type args: list """ - verbose_level = self._determine_verbose_level(args) - log_level_config = self.console_log_configs[verbose_level] + log_level = self._determine_log_level(args) + log_level_config = self.console_log_configs[log_level] root_logger = logging.getLogger() cli_logger = logging.getLogger(CLI_LOGGER_NAME) # Set the levels of the loggers to lowest level. @@ -140,16 +144,20 @@ def configure(self, args): self._init_logfile_handlers(root_logger, cli_logger) get_logger(__name__).debug("File logging enabled - writing logs to '%s'.", self.log_dir) - def _determine_verbose_level(self, args): + def _determine_log_level(self, args): """ Get verbose level by reading the arguments. """ - verbose_level = 0 - for arg in args: - if arg == CLILogging.VERBOSE_FLAG: - verbose_level += 1 - elif arg == CLILogging.DEBUG_FLAG: - verbose_level += 2 - # Use max verbose level if too much verbosity specified. - return min(verbose_level, len(self.console_log_configs) - 1) + self.cli_ctx.only_show_errors = False + if CLILogging.ONLY_SHOW_ERRORS_FLAG in args or \ + self.cli_ctx.config.get('core', 'only_show_errors', fallback=False): + if CLILogging.DEBUG_FLAG in args or CLILogging.VERBOSE_FLAG in args: + raise CLIError("--only-show-errors can't be used together with --debug or --verbose") + self.cli_ctx.only_show_errors = True + return 1 + if CLILogging.DEBUG_FLAG in args: + return 4 + if CLILogging.VERBOSE_FLAG in args: + return 3 + return 2 # default to show WARNINGs and above def _init_console_handlers(self, root_logger, cli_logger, log_level_config): root_logger.addHandler(_CustomStreamHandler(log_level_config['root'], @@ -179,6 +187,16 @@ def _get_log_dir(cli_ctx): @staticmethod def _get_console_log_configs(): return [ + # --only-show-critical [RESERVED] + { + CLI_LOGGER_NAME: logging.CRITICAL, + 'root': logging.CRITICAL + }, + # --only-show-errors + { + CLI_LOGGER_NAME: logging.ERROR, + 'root': logging.CRITICAL + }, # (default) { CLI_LOGGER_NAME: logging.WARNING, diff --git a/tests/test_deprecation.py b/tests/test_deprecation.py index 323f956..f6cf462 100644 --- a/tests/test_deprecation.py +++ b/tests/test_deprecation.py @@ -98,8 +98,8 @@ def test_deprecate_command_help_hidden(self): cmd3' instead. Arguments - -b [Required] : Allowed values: a, b, c. - --arg -a : Allowed values: 1, 2, 3. + -b [Required] : Allowed values: a, b, c. + --arg -a : Allowed values: 1, 2, 3. --arg3 """.format(self.cli_ctx.name) self.assertIn(expected, actual) diff --git a/tests/test_help.py b/tests/test_help.py index e1ca100..e05c6b0 100644 --- a/tests/test_help.py +++ b/tests/test_help.py @@ -295,6 +295,7 @@ def test_help_full_documentations(self): Global Arguments --debug : Increase logging verbosity to show all debug logs. --help -h : Show this help message and exit. + --only-show-errors : Only show errors, suppressing warnings. --output -o : Output format. Allowed values: json, jsonc, none, table, tsv, yaml, yamlc. Default: json. --query : JMESPath query string. See http://jmespath.org/ for more information and @@ -322,18 +323,19 @@ def test_help_with_param_specified(self): Long summary here. Still long summary. Arguments - -b [Required] : Allowed values: a, b, c. - --arg -a : Allowed values: 1, 2, 3. + -b [Required] : Allowed values: a, b, c. + --arg -a : Allowed values: 1, 2, 3. --arg3 Global Arguments - --debug : Increase logging verbosity to show all debug logs. - --help -h : Show this help message and exit. - --output -o : Output format. Allowed values: json, jsonc, none, table, tsv, yaml, yamlc. - Default: json. - --query : JMESPath query string. See http://jmespath.org/ for more information and - examples. - --verbose : Increase logging verbosity. Use --debug for full debug logs. + --debug : Increase logging verbosity to show all debug logs. + --help -h : Show this help message and exit. + --only-show-errors : Only show errors, suppressing warnings. + --output -o : Output format. Allowed values: json, jsonc, none, table, tsv, yaml, yamlc. + Default: json. + --query : JMESPath query string. See http://jmespath.org/ for more information and + examples. + --verbose : Increase logging verbosity. Use --debug for full debug logs. """ actual = io.getvalue() @@ -437,19 +439,20 @@ def register_globals(_, **kwargs): Long summary here. Still long summary. Arguments - -b [Required] : Allowed values: a, b, c. - --arg -a : Allowed values: 1, 2, 3. + -b [Required] : Allowed values: a, b, c. + --arg -a : Allowed values: 1, 2, 3. --arg3 Global Arguments - --debug : Increase logging verbosity to show all debug logs. - --exampl : This is a new global argument. - --help -h : Show this help message and exit. - --output -o : Output format. Allowed values: json, jsonc, none, table, tsv, yaml, yamlc. - Default: json. - --query : JMESPath query string. See http://jmespath.org/ for more information and - examples. - --verbose : Increase logging verbosity. Use --debug for full debug logs. + --debug : Increase logging verbosity to show all debug logs. + --exampl : This is a new global argument. + --help -h : Show this help message and exit. + --only-show-errors : Only show errors, suppressing warnings. + --output -o : Output format. Allowed values: json, jsonc, none, table, tsv, yaml, yamlc. + Default: json. + --query : JMESPath query string. See http://jmespath.org/ for more information and + examples. + --verbose : Increase logging verbosity. Use --debug for full debug logs. """ actual = io.getvalue() diff --git a/tests/test_log.py b/tests/test_log.py index 06f0c1e..4585103 100644 --- a/tests/test_log.py +++ b/tests/test_log.py @@ -13,6 +13,7 @@ from knack.events import EVENT_PARSER_GLOBAL_CREATE, EVENT_INVOKER_PRE_CMD_TBL_CREATE from knack.log import CLILogging, get_logger, CLI_LOGGER_NAME, _CustomStreamHandler +from knack.util import CLIError from tests.util import MockContext @@ -39,40 +40,51 @@ def setUp(self): self.mock_ctx = MockContext() self.cli_logging = CLILogging('clitest', cli_ctx=self.mock_ctx) - def test_determine_verbose_level_default(self): + def test_determine_log_level_default(self): argv = [] - actual_level = self.cli_logging._determine_verbose_level(argv) # pylint: disable=protected-access - expected_level = 0 + actual_level = self.cli_logging._determine_log_level(argv) # pylint: disable=protected-access + expected_level = 2 self.assertEqual(actual_level, expected_level) - def test_determine_verbose_level_verbose(self): + def test_determine_log_level_verbose(self): argv = ['--verbose'] - actual_level = self.cli_logging._determine_verbose_level(argv) # pylint: disable=protected-access - expected_level = 1 + actual_level = self.cli_logging._determine_log_level(argv) # pylint: disable=protected-access + expected_level = 3 self.assertEqual(actual_level, expected_level) - def test_determine_verbose_level_debug(self): + def test_determine_log_level_debug(self): argv = ['--debug'] - actual_level = self.cli_logging._determine_verbose_level(argv) # pylint: disable=protected-access - expected_level = 2 + actual_level = self.cli_logging._determine_log_level(argv) # pylint: disable=protected-access + expected_level = 4 self.assertEqual(actual_level, expected_level) - def test_determine_verbose_level_v_v_v_default(self): + def test_determine_log_level_v_v_v_default(self): argv = ['--verbose', '--debug'] - actual_level = self.cli_logging._determine_verbose_level(argv) # pylint: disable=protected-access - expected_level = 2 + actual_level = self.cli_logging._determine_log_level(argv) # pylint: disable=protected-access + expected_level = 4 self.assertEqual(actual_level, expected_level) - def test_determine_verbose_level_other_args_verbose(self): - argv = ['account', '--verbose'] - actual_level = self.cli_logging._determine_verbose_level(argv) # pylint: disable=protected-access + def test_determine_log_level_only_show_errors(self): + argv = ['--only-show-errors'] + actual_level = self.cli_logging._determine_log_level(argv) # pylint: disable=protected-access expected_level = 1 self.assertEqual(actual_level, expected_level) - def test_determine_verbose_level_other_args_debug(self): + def test_determine_log_level_all_flags(self): + argv = ['--verbose', '--debug', '--only-show-errors'] + with self.assertRaises(CLIError): + self.cli_logging._determine_log_level(argv) # pylint: disable=protected-access + + def test_determine_log_level_other_args_verbose(self): + argv = ['account', '--verbose'] + actual_level = self.cli_logging._determine_log_level(argv) # pylint: disable=protected-access + expected_level = 3 + self.assertEqual(actual_level, expected_level) + + def test_determine_log_level_other_args_debug(self): argv = ['account', '--debug'] - actual_level = self.cli_logging._determine_verbose_level(argv) # pylint: disable=protected-access - expected_level = 2 + actual_level = self.cli_logging._determine_log_level(argv) # pylint: disable=protected-access + expected_level = 4 self.assertEqual(actual_level, expected_level) def test_get_cli_logger(self): diff --git a/tests/test_preview.py b/tests/test_preview.py index f6e7084..ebb22dc 100644 --- a/tests/test_preview.py +++ b/tests/test_preview.py @@ -5,6 +5,7 @@ from __future__ import unicode_literals, print_function +import os import unittest try: import mock @@ -87,6 +88,27 @@ def test_preview_command_plain_execute(self): expected = "This command is in preview. It may be changed/removed in a future release." self.assertIn(expected, actual) + @redirect_io + def test_preview_command_plain_execute_only_show_error(self): + """ Ensure warning is suppressed when running preview command. """ + # Directly use --only-show-errors + self.cli_ctx.invoke('cmd1 -b b --only-show-errors'.split()) + actual = self.io.getvalue() + self.assertNotIn("preview", actual) + + # Apply --only-show-errors with config + self.cli_ctx.config.set_value('core', 'only_show_errors', 'True') + self.cli_ctx.invoke('cmd1 -b b'.split()) + actual = self.io.getvalue() + self.assertNotIn("preview", actual) + self.cli_ctx.config.set_value('core', 'only_show_errors', '') + + # Apply --only-show-errors with env var + os.environ["CLI_CORE_ONLY_SHOW_ERRORS"] = "True" + self.cli_ctx.invoke('cmd1 -b b'.split()) + actual = self.io.getvalue() + self.assertNotIn("preview", actual) + del(os.environ["CLI_CORE_ONLY_SHOW_ERRORS"]) class TestCommandGroupPreview(unittest.TestCase): @@ -202,5 +224,15 @@ def test_preview_arguments_execute(self): self.assertIn(action_expected, actual) + @redirect_io + def test_preview_arguments_execute_only_show_error(self): + """ Ensure warning is suppressed when using preview arguments. """ + self.cli_ctx.invoke('arg-test --arg1 foo --opt1 bar --only-show-errors'.split()) + actual = self.io.getvalue() + self.assertNotIn("preview", actual) + + action_expected = "Side-effect from some original action!" + self.assertIn(action_expected, actual) + if __name__ == '__main__': unittest.main() From f1fb5a1ef36c60f91192df766f10a6ab4b5e1d24 Mon Sep 17 00:00:00 2001 From: Jiashuo Li Date: Mon, 16 Mar 2020 20:17:15 +0800 Subject: [PATCH 2/2] Refactor --- knack/cli.py | 2 +- knack/log.py | 9 ++++++--- tests/test_log.py | 8 ++++++++ tests/test_preview.py | 8 ++------ 4 files changed, 17 insertions(+), 10 deletions(-) diff --git a/knack/cli.py b/knack/cli.py index 5469758..8a21474 100644 --- a/knack/cli.py +++ b/knack/cli.py @@ -91,7 +91,7 @@ def __init__(self, self.output = self.output_cls(cli_ctx=self) self.result = None self.query = query_cls(cli_ctx=self) - self.only_show_errors = False + self.only_show_errors = self.config.get('core', 'only_show_errors', fallback=False) self.enable_color = not self.config.get('core', 'no_color', fallback=False) @staticmethod diff --git a/knack/log.py b/knack/log.py index 4b6bf7f..5dbabf6 100644 --- a/knack/log.py +++ b/knack/log.py @@ -132,17 +132,20 @@ def configure(self, args): def _determine_log_level(self, args): """ Get verbose level by reading the arguments. """ - self.cli_ctx.only_show_errors = False - if CLILogging.ONLY_SHOW_ERRORS_FLAG in args or \ - self.cli_ctx.config.get('core', 'only_show_errors', fallback=False): + # arguments have higher precedence than config + if CLILogging.ONLY_SHOW_ERRORS_FLAG in args: if CLILogging.DEBUG_FLAG in args or CLILogging.VERBOSE_FLAG in args: raise CLIError("--only-show-errors can't be used together with --debug or --verbose") self.cli_ctx.only_show_errors = True return 1 if CLILogging.DEBUG_FLAG in args: + self.cli_ctx.only_show_errors = False return 4 if CLILogging.VERBOSE_FLAG in args: + self.cli_ctx.only_show_errors = False return 3 + if self.cli_ctx.only_show_errors: + return 1 return 2 # default to show WARNINGs and above def _init_console_handlers(self, root_logger, cli_logger, log_level_config): diff --git a/tests/test_log.py b/tests/test_log.py index 4585103..5791904 100644 --- a/tests/test_log.py +++ b/tests/test_log.py @@ -70,6 +70,14 @@ def test_determine_log_level_only_show_errors(self): expected_level = 1 self.assertEqual(actual_level, expected_level) + def test_determine_log_level_only_show_errors_config(self): + argv = [] + self.mock_ctx.only_show_errors = True + actual_level = self.cli_logging._determine_log_level(argv) # pylint: disable=protected-access + expected_level = 1 + self.assertEqual(actual_level, expected_level) + self.mock_ctx.only_show_errors = False + def test_determine_log_level_all_flags(self): argv = ['--verbose', '--debug', '--only-show-errors'] with self.assertRaises(CLIError): diff --git a/tests/test_preview.py b/tests/test_preview.py index 0a78099..868dab5 100644 --- a/tests/test_preview.py +++ b/tests/test_preview.py @@ -97,18 +97,14 @@ def test_preview_command_plain_execute_only_show_error(self): self.assertNotIn("preview", actual) # Apply --only-show-errors with config + self.cli_ctx.only_show_errors = True self.cli_ctx.config.set_value('core', 'only_show_errors', 'True') self.cli_ctx.invoke('cmd1 -b b'.split()) actual = self.io.getvalue() self.assertNotIn("preview", actual) self.cli_ctx.config.set_value('core', 'only_show_errors', '') + self.cli_ctx.only_show_errors = False - # Apply --only-show-errors with env var - os.environ["CLI_CORE_ONLY_SHOW_ERRORS"] = "True" - self.cli_ctx.invoke('cmd1 -b b'.split()) - actual = self.io.getvalue() - self.assertNotIn("preview", actual) - del(os.environ["CLI_CORE_ONLY_SHOW_ERRORS"]) @redirect_io @disable_color