From ba77ea9edcf2d99c6edd76adaa83017821babae3 Mon Sep 17 00:00:00 2001 From: Evil Ham Date: Mon, 1 Jun 2020 19:11:58 +0200 Subject: [PATCH] [UX] Add option to enable LogLevel-based coloured output. This makes it easier for new and experienced users to run cdist with higher verbosity levels, both to know that things are working as expected and to debug issues. Documentation has been modified accordingly and default behaviour is not changed. --- cdist/argparse.py | 31 ++++++++++++++++-- cdist/configuration.py | 11 +++++++ cdist/core/manifest.py | 1 + cdist/emulator.py | 3 ++ cdist/log.py | 26 ++++++++++++++- cdist/test/configuration/__init__.py | 28 +++++++++++++++++ configuration/cdist.cfg.skeleton | 6 ++++ docs/src/cdist-reference.rst.sh | 5 +++ docs/src/man1/cdist.rst | 47 +++++++++++++++++++--------- 9 files changed, 140 insertions(+), 18 deletions(-) diff --git a/cdist/argparse.py b/cdist/argparse.py index 611c484a..c30e2030 100644 --- a/cdist/argparse.py +++ b/cdist/argparse.py @@ -5,6 +5,7 @@ import logging import collections import functools import cdist.configuration +import cdist.log import cdist.preos import cdist.info @@ -88,6 +89,13 @@ def check_lower_bounded_int(value, lower_bound, name): return val +def colored_output_type(val): + boolean_states = cdist.configuration.ColoredOutputOption.BOOLEAN_STATES + if val not in boolean_states.keys(): + raise argparse.ArgumentError() + return boolean_states[val] + + def get_parsers(): global parser @@ -125,6 +133,15 @@ def get_parsers(): 'value.'), action='count', default=None) + parser['colored_output'] = argparse.ArgumentParser(add_help=False) + parser['colored_output'].add_argument( + '--colors', + help='Use a colored output for different log levels.' + 'It can be a boolean or "auto" (default) which enables this ' + 'feature if stdout is a tty and disables it otherwise.', + action='store', dest='colored_output', required=False, + type=colored_output_type) + parser['beta'] = argparse.ArgumentParser(add_help=False) parser['beta'].add_argument( '-b', '--beta', @@ -283,6 +300,7 @@ def get_parsers(): 'host', nargs='*', help='Host(s) to operate on.') parser['config'] = parser['sub'].add_parser( 'config', parents=[parser['loglevel'], parser['beta'], + parser['colored_output'], parser['common'], parser['config_main'], parser['inventory_common'], @@ -301,6 +319,7 @@ def get_parsers(): parser['add-host'] = parser['invsub'].add_parser( 'add-host', parents=[parser['loglevel'], parser['beta'], + parser['colored_output'], parser['common'], parser['inventory_common']]) parser['add-host'].add_argument( @@ -315,6 +334,7 @@ def get_parsers(): parser['add-tag'] = parser['invsub'].add_parser( 'add-tag', parents=[parser['loglevel'], parser['beta'], + parser['colored_output'], parser['common'], parser['inventory_common']]) parser['add-tag'].add_argument( @@ -346,6 +366,7 @@ def get_parsers(): parser['del-host'] = parser['invsub'].add_parser( 'del-host', parents=[parser['loglevel'], parser['beta'], + parser['colored_output'], parser['common'], parser['inventory_common']]) parser['del-host'].add_argument( @@ -363,6 +384,7 @@ def get_parsers(): parser['del-tag'] = parser['invsub'].add_parser( 'del-tag', parents=[parser['loglevel'], parser['beta'], + parser['colored_output'], parser['common'], parser['inventory_common']]) parser['del-tag'].add_argument( @@ -398,6 +420,7 @@ def get_parsers(): parser['list'] = parser['invsub'].add_parser( 'list', parents=[parser['loglevel'], parser['beta'], + parser['colored_output'], parser['common'], parser['inventory_common']]) parser['list'].add_argument( @@ -430,7 +453,7 @@ def get_parsers(): # Shell parser['shell'] = parser['sub'].add_parser( - 'shell', parents=[parser['loglevel']]) + 'shell', parents=[parser['loglevel'], parser['colored_output']]) parser['shell'].add_argument( '-s', '--shell', help=('Select shell to use, defaults to current shell. Used shell' @@ -495,9 +518,13 @@ def parse_and_configure(argv, singleton=True): log = logging.getLogger("cdist") + config = cfg.get_config() + if config.get('GLOBAL', {}).get('colored_output', False): + cdist.log.ColorFormatter.USE_COLORS = True + log.verbose("version %s" % cdist.VERSION) log.trace('command line args: {}'.format(cfg.command_line_args)) - log.trace('configuration: {}'.format(cfg.get_config())) + log.trace('configuration: {}'.format(config)) log.trace('configured args: {}'.format(args)) check_beta(vars(args)) diff --git a/cdist/configuration.py b/cdist/configuration.py index 1011a382..6f07c27f 100644 --- a/cdist/configuration.py +++ b/cdist/configuration.py @@ -27,6 +27,7 @@ import cdist.argparse import re import multiprocessing import logging +import sys class Singleton(type): @@ -47,6 +48,7 @@ _VERBOSITY_VALUES = ( _ARCHIVING_VALUES = ( 'tar', 'tgz', 'tbz2', 'txz', 'none', ) +_COLORED_OUTPUT_DEFAULT = sys.stdout.isatty() class OptionBase: @@ -246,9 +248,15 @@ class LogLevelOption(OptionBase): return VerbosityOption().translate(val) +class ColoredOutputOption(BooleanOption): + BOOLEAN_STATES = dict(configparser.ConfigParser.BOOLEAN_STATES, + auto=_COLORED_OUTPUT_DEFAULT) + + _ARG_OPTION_MAPPING = { 'beta': 'beta', 'cache_path_pattern': 'cache_path_pattern', + 'colored_output': 'colored_output', 'conf_dir': 'conf_dir', 'manifest': 'init_manifest', 'out_path': 'out_path', @@ -294,6 +302,7 @@ class Configuration(metaclass=Singleton): 'remote_shell': StringOption('remote_shell'), 'cache_path_pattern': StringOption('cache_path_pattern'), 'conf_dir': ConfDirOption(), + 'colored_output': ColoredOutputOption('colored_output'), 'init_manifest': StringOption('init_manifest'), 'out_path': StringOption('out_path'), 'remote_out_path': StringOption('remote_out_path'), @@ -319,6 +328,7 @@ class Configuration(metaclass=Singleton): 'CDIST_REMOTE_COPY': 'remote_copy', 'CDIST_INVENTORY_DIR': 'inventory_dir', 'CDIST_CACHE_PATH_PATTERN': 'cache_path_pattern', + 'CDIST_COLORED_OUTPUT': 'colored_output', '__cdist_log_level': 'verbosity', } ENV_VAR_BOOLEAN_OPTIONS = ('CDIST_BETA', ) @@ -332,6 +342,7 @@ class Configuration(metaclass=Singleton): } REQUIRED_DEFAULT_CONFIG_VALUES = { 'GLOBAL': { + 'colored_output': _COLORED_OUTPUT_DEFAULT, 'verbosity': 0, }, } diff --git a/cdist/core/manifest.py b/cdist/core/manifest.py index 8aeaf860..32520e49 100644 --- a/cdist/core/manifest.py +++ b/cdist/core/manifest.py @@ -119,6 +119,7 @@ class Manifest(object): '__cdist_log_level': util.log_level_env_var_val(self.log), '__cdist_log_level_name': util.log_level_name_env_var_val( self.log), + '__cdist_colored_log': str(cdist.log.ColorFormatter.USE_COLORS), } if dry_run: diff --git a/cdist/emulator.py b/cdist/emulator.py index 4800e2a3..87c9fe12 100644 --- a/cdist/emulator.py +++ b/cdist/emulator.py @@ -129,6 +129,9 @@ class Emulator(object): # if invalid __cdist_log_level value logging.root.setLevel(logging.WARNING) + colored_log = self.env.get('__cdist_colored_log', 'False') + cdist.log.ColorFormatter.USE_COLORS = colored_log == 'True' + self.log = logging.getLogger(self.target_host[0]) def commandline(self): diff --git a/cdist/log.py b/cdist/log.py index 2d0bef0b..5f2d8f53 100644 --- a/cdist/log.py +++ b/cdist/log.py @@ -50,6 +50,30 @@ def _trace(msg, *args, **kwargs): logging.trace = _trace +class ColorFormatter(logging.Formatter): + USE_COLORS = False + RESET = '\033[0m' + COLOR_MAP = { + 'ERROR': '\033[0;31m', + 'WARNING': '\033[0;33m', + 'INFO': '\033[0;94m', + 'VERBOSE': '\033[0;34m', + 'DEBUG': '\033[0;90m', + 'TRACE': '\033[0;37m', + } + + def __init__(self, msg): + super().__init__(msg) + + def format(self, record): + msg = super().format(record) + if self.USE_COLORS: + color = self.COLOR_MAP.get(record.levelname) + if color: + msg = color + msg + self.RESET + return msg + + class DefaultLog(logging.Logger): FORMAT = '%(levelname)s: %(message)s' @@ -66,7 +90,7 @@ class DefaultLog(logging.Logger): super().__init__(name) self.propagate = False - formatter = logging.Formatter(self.FORMAT) + formatter = ColorFormatter(self.FORMAT) self.addFilter(self) diff --git a/cdist/test/configuration/__init__.py b/cdist/test/configuration/__init__.py index 182868a6..07a73bda 100644 --- a/cdist/test/configuration/__init__.py +++ b/cdist/test/configuration/__init__.py @@ -28,10 +28,12 @@ import argparse from cdist import test import cdist.argparse as cap import logging +import sys my_dir = op.abspath(op.dirname(__file__)) fixtures = op.join(my_dir, 'fixtures') interpolation_config_file = op.join(fixtures, "interpolation-test.cfg") +colored_output_default = sys.stdout.isatty() def newConfigParser(): @@ -153,6 +155,7 @@ class ConfigurationTestCase(test.CdistTestCase): 'remote_shell': '/bin/sh', 'inventory_dir': '', 'cache_path_pattern': '', + 'colored_output': colored_output_default, 'conf_dir': '', 'init_manifest': '', 'out_path': '', @@ -184,6 +187,7 @@ class ConfigurationTestCase(test.CdistTestCase): 'remote_shell': '/bin/sh', 'inventory_dir': None, 'cache_path_pattern': None, + 'colored_output': colored_output_default, 'conf_dir': None, 'init_manifest': None, 'out_path': None, @@ -390,6 +394,7 @@ class ConfigurationTestCase(test.CdistTestCase): args = argparse.Namespace() expected_config_dict = { 'GLOBAL': { + 'colored_output': colored_output_default, 'verbosity': 0, }, } @@ -440,6 +445,7 @@ class ConfigurationTestCase(test.CdistTestCase): 'remote_shell': '/bin/sh', 'inventory_dir': None, 'cache_path_pattern': None, + 'colored_output': colored_output_default, 'conf_dir': None, 'init_manifest': None, 'out_path': None, @@ -515,6 +521,7 @@ class ConfigurationTestCase(test.CdistTestCase): 'remote_shell': '/usr/bin/sh', 'inventory_dir': '/var/db/cdist/inventory', 'cache_path_pattern': None, + 'colored_output': colored_output_default, 'conf_dir': ['/opt/cdist', ], 'init_manifest': None, 'out_path': None, @@ -556,6 +563,7 @@ class ConfigurationTestCase(test.CdistTestCase): 'remote_shell': '/bin/sh', 'inventory_dir': '', 'cache_path_pattern': '', + 'colored_output': colored_output_default, 'conf_dir': '', 'init_manifest': '', 'out_path': '', @@ -579,6 +587,7 @@ class ConfigurationTestCase(test.CdistTestCase): 'remote_shell': '/usr/bin/sh', 'inventory_dir': None, 'cache_path_pattern': None, + 'colored_output': colored_output_default, 'conf_dir': [ '/opt/cdist/conf', '/usr/local/share/cdist/conf', @@ -623,6 +632,7 @@ class ConfigurationTestCase(test.CdistTestCase): 'remote_shell': '/bin/sh', 'inventory_dir': '', 'cache_path_pattern': '', + 'colored_output': colored_output_default, 'conf_dir': '', 'init_manifest': '', 'out_path': '', @@ -645,6 +655,7 @@ class ConfigurationTestCase(test.CdistTestCase): 'local_shell': '/usr/bin/sh', 'remote_shell': '/usr/bin/sh', 'inventory_dir': '/var/db/cdist/inventory', + 'colored_output': colored_output_default, 'conf_dir': '/opt/cdist', 'remote_copy': 'myscp', 'remote_exec': 'myexec', @@ -663,6 +674,7 @@ class ConfigurationTestCase(test.CdistTestCase): 'remote_shell': '/usr/bin/sh', 'inventory_dir': '/var/db/cdist/inventory', 'cache_path_pattern': None, + 'colored_output': colored_output_default, 'conf_dir': [ '/opt/cdist/conf', '/usr/local/share/cdist/conf', @@ -694,6 +706,7 @@ class ConfigurationTestCase(test.CdistTestCase): } expected_config = { 'GLOBAL': { + 'colored_output': colored_output_default, 'verbosity': 0, }, } @@ -767,6 +780,7 @@ class ConfigurationTestCase(test.CdistTestCase): 'remote_shell': '/usr/bin/sh', 'inventory_dir': '/opt/sysadmin/cdist/inventory', 'cache_path_pattern': None, + 'colored_output': colored_output_default, 'conf_dir': [ '/opt/cdist/conf', '/usr/local/share/cdist/conf', @@ -865,6 +879,7 @@ class ConfigurationTestCase(test.CdistTestCase): 'remote_shell': '/usr/bin/sh', 'inventory_dir': '/var/db/cdist/inventory', 'cache_path_pattern': None, + 'colored_output': colored_output_default, 'conf_dir': [ '/opt/conf/cdist', ], @@ -964,6 +979,7 @@ class ConfigurationTestCase(test.CdistTestCase): 'remote_shell': '/usr/bin/sh', 'inventory_dir': '/var/db/cdist/inventory', 'cache_path_pattern': None, + 'colored_output': colored_output_default, 'conf_dir': [ '/opt/conf/cdist', ], @@ -1063,6 +1079,7 @@ class ConfigurationTestCase(test.CdistTestCase): 'remote_shell': '/usr/bin/sh', 'inventory_dir': '/var/db/cdist/inventory', 'cache_path_pattern': None, + 'colored_output': colored_output_default, 'conf_dir': [ '/opt/conf/cdist', ], @@ -1095,6 +1112,7 @@ class ConfigurationTestCase(test.CdistTestCase): 'beta': True, 'inventory_dir': '/var/db/cdist/inventory', 'cache_path_pattern': None, + 'colored_output': colored_output_default, 'conf_dir': [ '/opt/conf/cdist', ], @@ -1125,6 +1143,7 @@ class ConfigurationTestCase(test.CdistTestCase): expected_config_dict = { 'GLOBAL': { 'inventory_dir': None, + 'colored_output': colored_output_default, 'conf_dir': None, 'verbosity': 0, }, @@ -1148,6 +1167,7 @@ class ConfigurationTestCase(test.CdistTestCase): expected_config_dict = { 'GLOBAL': { + 'colored_output': colored_output_default, 'verbosity': cap.VERBOSE_DEBUG, }, } @@ -1185,6 +1205,7 @@ class ConfigurationTestCase(test.CdistTestCase): expected_config_dict = { 'GLOBAL': { + 'colored_output': colored_output_default, 'save_output_streams': True, 'verbosity': 0, }, @@ -1213,6 +1234,7 @@ class ConfigurationTestCase(test.CdistTestCase): expected_config_dict = { 'GLOBAL': { + 'colored_output': colored_output_default, 'save_output_streams': False, 'verbosity': 0, }, @@ -1241,6 +1263,7 @@ class ConfigurationTestCase(test.CdistTestCase): expected_config_dict = { 'GLOBAL': { + 'colored_output': colored_output_default, 'save_output_streams': False, 'verbosity': 0, }, @@ -1269,6 +1292,7 @@ class ConfigurationTestCase(test.CdistTestCase): expected_config_dict = { 'GLOBAL': { + 'colored_output': colored_output_default, 'save_output_streams': False, 'verbosity': 0, }, @@ -1308,6 +1332,7 @@ class ConfigurationTestCase(test.CdistTestCase): expected_config_dict = { 'GLOBAL': { + 'colored_output': colored_output_default, 'timestamp': True, 'verbosity': 0, }, @@ -1336,6 +1361,7 @@ class ConfigurationTestCase(test.CdistTestCase): expected_config_dict = { 'GLOBAL': { + 'colored_output': colored_output_default, 'timestamp': True, 'verbosity': 0, }, @@ -1364,6 +1390,7 @@ class ConfigurationTestCase(test.CdistTestCase): expected_config_dict = { 'GLOBAL': { + 'colored_output': colored_output_default, 'timestamp': False, 'verbosity': 0, }, @@ -1392,6 +1419,7 @@ class ConfigurationTestCase(test.CdistTestCase): expected_config_dict = { 'GLOBAL': { + 'colored_output': colored_output_default, 'timestamp': False, 'verbosity': 0, }, diff --git a/configuration/cdist.cfg.skeleton b/configuration/cdist.cfg.skeleton index 91c5ab02..f2a09064 100644 --- a/configuration/cdist.cfg.skeleton +++ b/configuration/cdist.cfg.skeleton @@ -13,6 +13,12 @@ # Specify cache path pattern. # cache_path_pattern = %h # +# colored_output +# Use a colored output for different log levels. +# It can be a boolean or 'auto' (default) which enables this feature if +# stdout is a tty and disables it otherwise. +# colored_output = auto +# # conf_dir # List of configuration directories separated with the character conventionally # used by the operating system to separate search path components (as in PATH), diff --git a/docs/src/cdist-reference.rst.sh b/docs/src/cdist-reference.rst.sh index e77d98f6..3b997f63 100755 --- a/docs/src/cdist-reference.rst.sh +++ b/docs/src/cdist-reference.rst.sh @@ -344,6 +344,11 @@ CDIST_INVENTORY_DIR CDIST_BETA Enable beta functionalities. +CDIST_COLORED_OUTPUT + Use a colored output for different log levels. + It can be a boolean or 'auto' (default) which enables this feature if + stdout is a tty and disables it otherwise. + CDIST_CACHE_PATH_PATTERN Custom cache path pattern. eof diff --git a/docs/src/man1/cdist.rst b/docs/src/man1/cdist.rst index 38248821..4c34c4b7 100644 --- a/docs/src/man1/cdist.rst +++ b/docs/src/man1/cdist.rst @@ -15,8 +15,9 @@ SYNOPSIS cdist banner [-h] [-l LOGLEVEL] [-q] [-v] - cdist config [-h] [-l LOGLEVEL] [-q] [-v] [-b] [-g CONFIG_FILE] [-4] - [-6] [-C CACHE_PATH_PATTERN] [-c CONF_DIR] [-i MANIFEST] + cdist config [-h] [-l LOGLEVEL] [-q] [-v] [-b] + [--colors COLORED_OUTPUT] [-g CONFIG_FILE] [-4] [-6] + [-C CACHE_PATH_PATTERN] [-c CONF_DIR] [-i MANIFEST] [-j [JOBS]] [-n] [-o OUT_PATH] [-P] [-R [{tar,tgz,tbz2,txz}]] [-r REMOTE_OUT_PATH] [--remote-copy REMOTE_COPY] [--remote-exec REMOTE_EXEC] @@ -24,8 +25,9 @@ SYNOPSIS [-p [HOST_MAX]] [-s] [-t] [host [host ...]] - cdist install [-h] [-l LOGLEVEL] [-q] [-v] [-b] [-g CONFIG_FILE] [-4] - [-6] [-C CACHE_PATH_PATTERN] [-c CONF_DIR] [-i MANIFEST] + cdist install [-h] [-l LOGLEVEL] [-q] [-v] [-b] + [--colors COLORED_OUTPUT] [-g CONFIG_FILE] [-4] [-6] + [-C CACHE_PATH_PATTERN] [-c CONF_DIR] [-i MANIFEST] [-j [JOBS]] [-n] [-o OUT_PATH] [-P] [-R [{tar,tgz,tbz2,txz}]] [-r REMOTE_OUT_PATH] [--remote-copy REMOTE_COPY] [--remote-exec REMOTE_EXEC] @@ -36,26 +38,29 @@ SYNOPSIS cdist inventory [-h] {add-host,add-tag,del-host,del-tag,list} ... cdist inventory add-host [-h] [-l LOGLEVEL] [-q] [-v] [-b] - [-g CONFIG_FILE] [-I INVENTORY_DIR] - [-f HOSTFILE] + [--colors COLORED_OUTPUT] [-g CONFIG_FILE] + [-I INVENTORY_DIR] [-f HOSTFILE] [host [host ...]] cdist inventory add-tag [-h] [-l LOGLEVEL] [-q] [-v] [-b] - [-g CONFIG_FILE] [-I INVENTORY_DIR] - [-f HOSTFILE] [-T TAGFILE] [-t TAGLIST] + [--colors COLORED_OUTPUT] [-g CONFIG_FILE] + [-I INVENTORY_DIR] [-f HOSTFILE] [-T TAGFILE] + [-t TAGLIST] [host [host ...]] cdist inventory del-host [-h] [-l LOGLEVEL] [-q] [-v] [-b] - [-g CONFIG_FILE] [-I INVENTORY_DIR] [-a] - [-f HOSTFILE] + [--colors COLORED_OUTPUT] [-g CONFIG_FILE] + [-I INVENTORY_DIR] [-a] [-f HOSTFILE] [host [host ...]] cdist inventory del-tag [-h] [-l LOGLEVEL] [-q] [-v] [-b] - [-g CONFIG_FILE] [-I INVENTORY_DIR] [-a] - [-f HOSTFILE] [-T TAGFILE] [-t TAGLIST] + [--colors COLORED_OUTPUT] [-g CONFIG_FILE] + [-I INVENTORY_DIR] [-a] [-f HOSTFILE] + [-T TAGFILE] [-t TAGLIST] [host [host ...]] - cdist inventory list [-h] [-l LOGLEVEL] [-q] [-v] [-b] [-g CONFIG_FILE] + cdist inventory list [-h] [-l LOGLEVEL] [-q] [-v] [-b] + [--colors COLORED_OUTPUT] [-g CONFIG_FILE] [-I INVENTORY_DIR] [-a] [-f HOSTFILE] [-H] [-t] [host [host ...]] @@ -84,9 +89,11 @@ SYNOPSIS [-S SCRIPT] [-s SUITE] [-y REMOTE_COPY] target_dir - cdist shell [-h] [-l LOGLEVEL] [-q] [-v] [-s SHELL] + cdist shell [-h] [-l LOGLEVEL] [-q] [-v] [--colors COLORED_OUTPUT] + [-s SHELL] - cdist info [-h] [-a] [-c CONF_DIR] [-e] [-F] [-f] [-g CONFIG_FILE] [-t] [pattern] + cdist info [-h] [-a] [-c CONF_DIR] [-e] [-F] [-f] [-g CONFIG_FILE] [-t] + [pattern] DESCRIPTION @@ -104,6 +111,11 @@ All commands accept the following options: **-h, --help** Show the help screen. +**--colors COLORED_OUTPUT** + Use a colored output for different log levels.It can + be a boolean or "auto" (default) which enables this + feature if stdout is a tty and disables it otherwise. + **-l LOGLEVEL, --log-level LOGLEVEL** Set the specified verbosity level. The levels, in order from the lowest to the highest, are: ERROR (-1), @@ -893,6 +905,11 @@ CDIST_BETA CDIST_CACHE_PATH_PATTERN Custom cache path pattern. +CDIST_COLORED_OUTPUT + Use a colored output for different log levels. + It can be a boolean or 'auto' (default) which enables this feature if + stdout is a tty and disables it otherwise. + CDIST_CONFIG_FILE Custom configuration file.