Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Log] Support multiple CLI loggers #227

Merged
merged 2 commits into from
Dec 29, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions HISTORY.rst
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,11 @@
Release History
===============

0.8.0rc2
++++++++

* Support multiple cli loggers by adding more logger names to `knack.log.cli_logger_names` list (#227)

0.8.0rc1
++++++++
* Make config item names case-insensitive (#220)
Expand Down
58 changes: 32 additions & 26 deletions knack/log.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,10 @@
from .events import EVENT_PARSER_GLOBAL_CREATE


CLI_LOGGER_NAME = 'knack'
CLI_LOGGER_NAME = 'cli'
# Add more logger names to this list so that ERROR, WARNING, INFO logs from these loggers can also be displayed
# without --debug flag.
cli_logger_names = [CLI_LOGGER_NAME]


class CliLogLevel(IntEnum):
Expand All @@ -34,7 +37,7 @@ def get_logger(module_name=None):
:rtype: logger
"""
if module_name:
logger_name = module_name
logger_name = '{}.{}'.format(CLI_LOGGER_NAME, module_name)
else:
logger_name = CLI_LOGGER_NAME
return logging.getLogger(logger_name)
Expand Down Expand Up @@ -79,7 +82,7 @@ def format(self, record):
return msg


class CLILogging(object): # pylint: disable=too-many-instance-attributes
class CLILogging: # pylint: disable=too-many-instance-attributes

DEBUG_FLAG = '--debug'
VERBOSE_FLAG = '--verbose'
Expand All @@ -97,20 +100,17 @@ def on_global_arguments(_, **kwargs):
action='store_true',
help='Only show errors, suppressing warnings.')

def __init__(self, name, cli_ctx=None, cli_logger_name=None):
def __init__(self, name, cli_ctx=None):
"""

:param name: The name to be used for log files
:type name: str
:param cli_ctx: CLI Context
:type cli_ctx: knack.cli.CLI
:param cli_logger_name: Used to override CLI_LOGGER_NAME
:type cli_logger_name: str
"""
from .cli import CLI
if cli_ctx is not None and not isinstance(cli_ctx, CLI):
raise CtxTypeError(cli_ctx)
self.cli_logger_name = cli_logger_name or CLI_LOGGER_NAME
self.logfile_name = '{}.log'.format(name)
self.file_log_enabled = CLILogging._is_file_log_enabled(cli_ctx)
self.log_dir = CLILogging._get_log_dir(cli_ctx)
Expand All @@ -129,19 +129,23 @@ def configure(self, args):
self.log_level = self._determine_log_level(args)
console_log_levels = self._get_console_log_levels()
console_log_formats = self._get_console_log_formats()

root_logger = logging.getLogger()
cli_logger = logging.getLogger(self.cli_logger_name)
# Set the levels of the loggers to lowest level.
# Handlers can override by choosing a higher level.
root_logger.setLevel(logging.DEBUG)
cli_logger.setLevel(logging.DEBUG)
cli_logger.propagate = False
if root_logger.handlers and cli_logger.handlers:
# loggers already configured

cli_loggers = [logging.getLogger(logger_name) for logger_name in cli_logger_names]
for cli_logger in cli_loggers:
cli_logger.setLevel(logging.DEBUG)
cli_logger.propagate = False

if root_logger.handlers:
# handlers already configured
return
Comment on lines +143 to 145
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it enough when root_logger.handlers is set?
In previous code, both root_logger.handlers and cli_logger.handlers are checked.
Should we check all cli_logger.handlers for cli_logger in cli_loggers?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note really necessary, as root logger and cli logger are always set at the same time.

knack/knack/log.py

Lines 171 to 177 in 30592e0

root_logger.addHandler(_CustomStreamHandler(log_levels['root'],
log_formats['root'],
self.cli_ctx.enable_color))
cli_logger_console_handler = _CustomStreamHandler(log_levels['cli'], log_formats['cli'],
self.cli_ctx.enable_color)
for cli_logger in cli_loggers:
cli_logger.addHandler(cli_logger_console_handler)

self._init_console_handlers(root_logger, cli_logger, console_log_levels, console_log_formats)
self._init_console_handlers(root_logger, cli_loggers, console_log_levels, console_log_formats)
if self.file_log_enabled:
self._init_logfile_handlers(root_logger, cli_logger)
self._init_logfile_handlers(root_logger, cli_loggers)
get_logger(__name__).debug("File logging enabled - writing logs to '%s'.", self.log_dir)

def _determine_log_level(self, args):
Expand All @@ -163,15 +167,16 @@ def _determine_log_level(self, args):
return CliLogLevel.ERROR
return CliLogLevel.WARNING # default to show WARNINGs and above

def _init_console_handlers(self, root_logger, cli_logger, log_levels, log_formats):
def _init_console_handlers(self, root_logger, cli_loggers, log_levels, log_formats):
root_logger.addHandler(_CustomStreamHandler(log_levels['root'],
log_formats['root'],
self.cli_ctx.enable_color))
cli_logger.addHandler(_CustomStreamHandler(log_levels[self.cli_logger_name],
log_formats[self.cli_logger_name],
self.cli_ctx.enable_color))
cli_logger_console_handler = _CustomStreamHandler(log_levels['cli'], log_formats['cli'],
self.cli_ctx.enable_color)
for cli_logger in cli_loggers:
cli_logger.addHandler(cli_logger_console_handler)

def _init_logfile_handlers(self, root_logger, cli_logger):
def _init_logfile_handlers(self, root_logger, cli_loggers):
ensure_dir(self.log_dir)
log_file_path = os.path.join(self.log_dir, self.logfile_name)
from logging.handlers import RotatingFileHandler
Expand All @@ -180,7 +185,8 @@ def _init_logfile_handlers(self, root_logger, cli_logger):
logfile_handler.setFormatter(lfmt)
logfile_handler.setLevel(logging.DEBUG)
root_logger.addHandler(logfile_handler)
cli_logger.addHandler(logfile_handler)
for cli_logger in cli_loggers:
cli_logger.addHandler(logfile_handler)

@staticmethod
def _is_file_log_enabled(cli_ctx):
Expand All @@ -200,27 +206,27 @@ def _get_console_log_levels(self):
level_list = [
# --only-show-critical [RESERVED]
{
self.cli_logger_name: logging.CRITICAL,
'cli': logging.CRITICAL,
'root': logging.CRITICAL
},
# --only-show-errors
{
self.cli_logger_name: logging.ERROR,
'cli': logging.ERROR,
'root': logging.CRITICAL
},
# (default)
{
self.cli_logger_name: logging.WARNING,
'cli': logging.WARNING,
'root': logging.CRITICAL,
},
# --verbose
{
self.cli_logger_name: logging.INFO,
'cli': logging.INFO,
'root': logging.CRITICAL,
},
# --debug
{
self.cli_logger_name: logging.DEBUG,
'cli': logging.DEBUG,
'root': logging.DEBUG,
}]
return level_list[self.log_level]
Expand Down Expand Up @@ -250,6 +256,6 @@ def _get_console_log_formats(self):
log_format = ': '.join(elements)
return {
# Even though these loggers use the same format, keep the dict for further tuning
self.cli_logger_name: log_format,
'cli': log_format,
'root': log_format
}
2 changes: 1 addition & 1 deletion setup.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
from codecs import open
from setuptools import setup, find_packages

VERSION = '0.8.0rc1'
VERSION = '0.8.0rc2'

DEPENDENCIES = [
'argcomplete',
Expand Down
22 changes: 10 additions & 12 deletions tests/test_log.py
Original file line number Diff line number Diff line change
Expand Up @@ -101,68 +101,66 @@ def test_get_cli_logger(self):

def test_get_module_logger(self):
module_logger = get_logger('a.module')
self.assertEqual(module_logger.name, 'a.module')
module_logger = get_logger()
self.assertEqual(module_logger.name, 'knack')
self.assertEqual(module_logger.name, 'cli.a.module')

def test_get_console_log_levels(self):
# CRITICAL
self.cli_logging.log_level = 0
levels = self.cli_logging._get_console_log_levels()
expected = {'knack': 50, 'root': 50}
expected = {'cli': 50, 'root': 50}
self.assertEqual(levels, expected)

# ERROR
self.cli_logging.log_level = 1
levels = self.cli_logging._get_console_log_levels()
expected = {'knack': 40, 'root': 50}
expected = {'cli': 40, 'root': 50}
self.assertEqual(levels, expected)

# WARNING
self.cli_logging.log_level = 2
levels = self.cli_logging._get_console_log_levels()
expected = {'knack': 30, 'root': 50}
expected = {'cli': 30, 'root': 50}
self.assertEqual(levels, expected)

# INFO
self.cli_logging.log_level = 3
levels = self.cli_logging._get_console_log_levels()
expected = {'knack': 20, 'root': 50}
expected = {'cli': 20, 'root': 50}
self.assertEqual(levels, expected)

# DEBUG
self.cli_logging.log_level = 4
levels = self.cli_logging._get_console_log_levels()
expected = {'knack': 10, 'root': 10}
expected = {'cli': 10, 'root': 10}
self.assertEqual(levels, expected)

def test_get_console_log_formats(self):
# DEBUG level, color enabled
self.cli_logging.log_level = 4
self.cli_logging.cli_ctx.enable_color = True
formats = self.cli_logging._get_console_log_formats()
expected = {'knack': '%(name)s: %(message)s', 'root': '%(name)s: %(message)s'}
expected = {'cli': '%(name)s: %(message)s', 'root': '%(name)s: %(message)s'}
self.assertEqual(formats, expected)

# DEBUG level, color disabled
self.cli_logging.log_level = 4
self.cli_logging.cli_ctx.enable_color = False
formats = self.cli_logging._get_console_log_formats()
expected = {'knack': '%(levelname)s: %(name)s: %(message)s', 'root': '%(levelname)s: %(name)s: %(message)s'}
expected = {'cli': '%(levelname)s: %(name)s: %(message)s', 'root': '%(levelname)s: %(name)s: %(message)s'}
self.assertEqual(formats, expected)

# WARNING level, color enabled
self.cli_logging.log_level = 2
self.cli_logging.cli_ctx.enable_color = True
formats = self.cli_logging._get_console_log_formats()
expected = {'knack': '%(message)s', 'root': '%(message)s'}
expected = {'cli': '%(message)s', 'root': '%(message)s'}
self.assertEqual(formats, expected)

# WARNING level, color disabled
self.cli_logging.log_level = 2
self.cli_logging.cli_ctx.enable_color = False
formats = self.cli_logging._get_console_log_formats()
expected = {'knack': '%(levelname)s: %(message)s', 'root': '%(levelname)s: %(message)s'}
expected = {'cli': '%(levelname)s: %(message)s', 'root': '%(levelname)s: %(message)s'}
self.assertEqual(formats, expected)


Expand Down