Volans has uploaded a new change for review. (
https://gerrit.wikimedia.org/r/386399 )
Change subject: Logging: uniform loggers
......................................................................
Logging: uniform loggers
* Use proper hierarchical loggers across the project
* [Breaking API change] Remove optional parameter logger from all
classes where it was accepted, the classes instantiate the proper
logger based on the current module and class name.
* For classes inherited from a base abstract class, the logger is
defined only in the base abstract class, with the name of the concrete
class that is calling it.
* Changed CLI logging format to take advantage of the hirarchical
logging.
Bug: T179002
Change-Id: I44eaed95ae4e7d12f16a03029e7c753bbefccb71
---
M cumin/backends/__init__.py
M cumin/backends/openstack.py
M cumin/backends/puppetdb.py
M cumin/cli.py
M cumin/query.py
M cumin/tests/unit/backends/external/__init__.py
M cumin/tests/unit/test_cli.py
M cumin/transport.py
M cumin/transports/__init__.py
M cumin/transports/clustershell.py
10 files changed, 46 insertions(+), 50 deletions(-)
git pull ssh://gerrit.wikimedia.org:29418/operations/software/cumin
refs/changes/99/386399/1
diff --git a/cumin/backends/__init__.py b/cumin/backends/__init__.py
index 19a6c0e..9961f1c 100644
--- a/cumin/backends/__init__.py
+++ b/cumin/backends/__init__.py
@@ -27,15 +27,14 @@
""":py:class:`pyparsing.ParserElement`: derived classes must define their
own pyparsing grammar and set this class
attribute accordingly."""
- def __init__(self, config, logger=None):
+ def __init__(self, config):
"""Query constructor.
Arguments:
config (dict): a dictionary with the parsed configuration file.
- logger (logging.Logger, optional): an optional logger instance.
"""
self.config = config
- self.logger = logger or logging.getLogger(__name__)
+ self.logger = logging.getLogger('.'.join((self.__module__,
self.__class__.__name__)))
self.logger.trace('Backend {name} created with config:
{config}'.format(
name=type(self).__name__, config=config))
@@ -92,13 +91,13 @@
operators.
"""
- def __init__(self, config, logger=None):
+ def __init__(self, config):
"""Query aggregator constructor, initialize the stack.
:Parameters:
according to parent :py:meth:`cumin.backends.BaseQuery.__init__`.
"""
- super(BaseQueryAggregator, self).__init__(config, logger=logger)
+ super(BaseQueryAggregator, self).__init__(config)
self.stack = None
self.stack_pointer = None
diff --git a/cumin/backends/openstack.py b/cumin/backends/openstack.py
index b2e1b71..56e0235 100644
--- a/cumin/backends/openstack.py
+++ b/cumin/backends/openstack.py
@@ -100,14 +100,14 @@
grammar = grammar()
""":py:class:`pyparsing.ParserElement`: load the grammar parser only once
in a singleton-like way."""
- def __init__(self, config, logger=None):
+ def __init__(self, config):
"""Override parent class constructor for specific setup.
:Parameters:
according to parent :py:meth:`cumin.backends.BaseQuery.__init__`.
"""
- super(OpenStackQuery, self).__init__(config, logger=logger)
+ super(OpenStackQuery, self).__init__(config)
self.openstack_config = self.config.get('openstack', {})
self.search_project = None
self.search_params = self._get_default_search_params()
diff --git a/cumin/backends/puppetdb.py b/cumin/backends/puppetdb.py
index 7d3168a..1e76a9b 100644
--- a/cumin/backends/puppetdb.py
+++ b/cumin/backends/puppetdb.py
@@ -140,14 +140,14 @@
grammar = grammar()
""":py:class:`pyparsing.ParserElement`: load the grammar parser only once
in a singleton-like way."""
- def __init__(self, config, logger=None):
+ def __init__(self, config):
"""Query constructor for the PuppetDB backend.
:Parameters:
according to parent :py:meth:`cumin.backends.BaseQuery.__init__`.
"""
- super(PuppetDBQuery, self).__init__(config, logger=logger)
+ super(PuppetDBQuery, self).__init__(config)
self.grouped_tokens = None
self.current_group = self.grouped_tokens
self._endpoint = None
diff --git a/cumin/cli.py b/cumin/cli.py
index 84f70e2..cb75b5d 100644
--- a/cumin/cli.py
+++ b/cumin/cli.py
@@ -188,19 +188,20 @@
if not os.path.exists(file_path):
os.makedirs(file_path, 0770)
- log_formatter = logging.Formatter(
- fmt='%(asctime)s [%(process)s] (%(levelname)s %(filename)s:%(lineno)s
in %(funcName)s) %(message)s')
+ log_formatter = logging.Formatter(fmt='%(asctime)s [%(levelname)s
%(process)s %(name)s.%(funcName)s] %(message)s')
log_handler = RotatingFileHandler(filename, maxBytes=(5 * (1024**2)),
backupCount=30)
log_handler.setFormatter(log_formatter)
- logger.addHandler(log_handler)
- logger.raiseExceptions = False
+
+ root_logger = logging.getLogger()
+ root_logger.addHandler(log_handler)
+ root_logger.raiseExceptions = False
if trace:
- logger.setLevel(cumin.LOGGING_TRACE_LEVEL_NUMBER)
+ root_logger.setLevel(cumin.LOGGING_TRACE_LEVEL_NUMBER)
elif debug:
- logger.setLevel(logging.DEBUG)
+ root_logger.setLevel(logging.DEBUG)
else:
- logger.setLevel(logging.INFO)
+ root_logger.setLevel(logging.INFO)
def sigint_handler(*args): # pylint: disable=unused-argument
@@ -262,7 +263,7 @@
args: ArgumentParser instance with parsed command line arguments
config: a dictionary with the parsed configuration file
"""
- hosts = query.Query(config, logger=logger).execute(args.hosts)
+ hosts = query.Query(config).execute(args.hosts)
if not hosts:
stderr('No hosts found that matches the query')
@@ -337,8 +338,8 @@
if not hosts:
return 0
- target = transports.Target(hosts, batch_size=args.batch_size,
batch_sleep=args.batch_sleep, logger=logger)
- worker = transport.Transport.new(config, target, logger=logger)
+ target = transports.Target(hosts, batch_size=args.batch_size,
batch_sleep=args.batch_sleep)
+ worker = transport.Transport.new(config, target)
ok_codes = None
if args.ignore_exit_codes:
diff --git a/cumin/query.py b/cumin/query.py
index 839dc42..8f005e0 100644
--- a/cumin/query.py
+++ b/cumin/query.py
@@ -20,17 +20,17 @@
>>> import cumin
>>> from cumin.query import Query
>>> config = cumin.Config()
- >>> hosts = Query(config, logger=logger).execute(query_string)
+ >>> hosts = Query(config).execute(query_string)
"""
- def __init__(self, config, logger=None):
+ def __init__(self, config):
"""Query constructor, initialize the registered backends.
:Parameters:
according to parent
:py:meth:`cumin.backends.BaseQueryAggregator.__init__`.
"""
- super(Query, self).__init__(config, logger=logger)
+ super(Query, self).__init__(config)
external = self.config.get('plugins', {}).get('backends', [])
self.registered_backends =
grammar.get_registered_backends(external=external)
self.grammar = grammar.grammar(self.registered_backends.keys())
@@ -89,7 +89,7 @@
raise InvalidQueryError("Default backend '{name}' is not
registered: {backends}".format(
name=self.config['default_backend'],
backends=self.registered_backends))
- query = backend.cls(self.config, logger=self.logger)
+ query = backend.cls(self.config)
return query.execute(query_string)
@@ -115,7 +115,7 @@
if 'backend' in token_dict and 'query' in token_dict:
element = self._get_stack_element()
- query =
self.registered_backends[token_dict['backend']].cls(self.config,
logger=self.logger)
+ query =
self.registered_backends[token_dict['backend']].cls(self.config)
element['hosts'] = query.execute(token_dict['query'])
if 'bool' in token_dict:
element['bool'] = token_dict['bool']
diff --git a/cumin/tests/unit/backends/external/__init__.py
b/cumin/tests/unit/backends/external/__init__.py
index 2c2a8aa..374ded9 100644
--- a/cumin/tests/unit/backends/external/__init__.py
+++ b/cumin/tests/unit/backends/external/__init__.py
@@ -25,14 +25,14 @@
grammar = grammar()
""":py:class:`pyparsing.ParserElement`: load the grammar parser only once
in a singleton-like way."""
- def __init__(self, config, logger=None):
+ def __init__(self, config):
"""Query constructor for the test external backend.
:Parameters:
according to parent :py:meth:`cumin.backends.BaseQuery.__init__`.
"""
- super(ExternalBackendQuery, self).__init__(config, logger=logger)
+ super(ExternalBackendQuery, self).__init__(config)
self.hosts = NodeSet()
def _execute(self):
diff --git a/cumin/tests/unit/test_cli.py b/cumin/tests/unit/test_cli.py
index b61d5ac..5ab1a76 100644
--- a/cumin/tests/unit/test_cli.py
+++ b/cumin/tests/unit/test_cli.py
@@ -70,21 +70,21 @@
@mock.patch('cumin.cli.os')
@mock.patch('cumin.cli.RotatingFileHandler')
[email protected]('cumin.cli.logger')
-def test_setup_logging(logging, file_handler, mocked_os):
[email protected]('cumin.cli.logging.getLogger')
+def test_setup_logging(mocked_get_logger, file_handler, mocked_os):
"""Calling setup_logging() should properly setup the logger."""
mocked_os.path.exists.return_value = False
cli.setup_logging('/path/to/filename')
- logging.setLevel.assert_called_with(INFO)
+ assert mock.call().setLevel(INFO) in mocked_get_logger.mock_calls
assert file_handler.called
mocked_os.path.exists.return_value = True
cli.setup_logging('filename', debug=True)
- logging.setLevel.assert_called_with(DEBUG)
+ assert mock.call().setLevel(DEBUG) in mocked_get_logger.mock_calls
mocked_os.path.exists.return_value = True
cli.setup_logging('filename', trace=True)
- logging.setLevel.assert_called_with(LOGGING_TRACE_LEVEL_NUMBER)
+ assert mock.call().setLevel(LOGGING_TRACE_LEVEL_NUMBER) in
mocked_get_logger.mock_calls
@mock.patch('cumin.cli.stderr')
@@ -206,7 +206,6 @@
cli.run(args, config)
assert transport.new.call_args[0][0] is config
assert isinstance(transport.new.call_args[0][1], transports.Target)
- assert transport.new.call_args[1]['logger'] is cli.logger
assert stderr.called
diff --git a/cumin/transport.py b/cumin/transport.py
index bdca28a..25c268e 100644
--- a/cumin/transport.py
+++ b/cumin/transport.py
@@ -9,13 +9,12 @@
"""Transport factory class."""
@staticmethod
- def new(config, target, logger=None):
+ def new(config, target):
"""Create a transport worker class based on the configuration
(`factory`).
Arguments:
config (dict): the configuration dictionary.
target (cumin.transports.Target): a Target instance.
- logger (logging.Logger, optional): an optional logger instance.
Returns:
BaseWorker: the created worker instance for the configured
transport.
@@ -31,7 +30,7 @@
try:
module =
importlib.import_module('cumin.transports.{transport}'.format(transport=config['transport']))
- return module.worker_class(config, target, logger=logger)
+ return module.worker_class(config, target)
except (AttributeError, ImportError) as e:
e.message = "Unable to load worker class for transport
'{transport}': {msg}".format(
transport=config['transport'], msg=repr(e))
diff --git a/cumin/transports/__init__.py b/cumin/transports/__init__.py
index 153f177..d046cc2 100644
--- a/cumin/transports/__init__.py
+++ b/cumin/transports/__init__.py
@@ -325,7 +325,7 @@
class Target(object):
"""Targets management class."""
- def __init__(self, hosts, batch_size=None, batch_sleep=None, logger=None):
+ def __init__(self, hosts, batch_size=None, batch_sleep=None):
"""Constructor, inizialize the Target with the list of hosts and
additional parameters.
Arguments:
@@ -337,13 +337,12 @@
It must be a positive integer or :py:data:`None` to unset it.
batch_sleep (int, optional): sleep time in seconds between the end
of execution of one host in the
batch and the start in the next host. It must be a positive
float or None to unset it.
- logger (logging.Logger, optional): a logger instance.
Raises:
cumin.transports.WorkerError: if the `hosts` parameter is invalid.
"""
- self.logger = logger or logging.getLogger(__name__)
+ self.logger = logging.getLogger('.'.join((self.__module__,
self.__class__.__name__)))
if isinstance(hosts, NodeSet):
self.hosts = hosts
@@ -411,17 +410,16 @@
__metaclass__ = ABCMeta
- def __init__(self, config, target, logger=None):
+ def __init__(self, config, target):
"""Worker constructor. Setup environment variables and initialize
properties.
Arguments:
config (dict): a dictionary with the parsed configuration file.
target (Target): a Target instance.
- logger (logging.Logger, optional): an optional logger instance.
"""
self.config = config
self.target = target
- self.logger = logger or logging.getLogger(__name__)
+ self.logger = logging.getLogger('.'.join((self.__module__,
self.__class__.__name__)))
self.logger.trace('Transport {name} created with config:
{config}'.format(
name=type(self).__name__, config=config))
diff --git a/cumin/transports/clustershell.py b/cumin/transports/clustershell.py
index d5ba468..36f5c8f 100644
--- a/cumin/transports/clustershell.py
+++ b/cumin/transports/clustershell.py
@@ -16,13 +16,13 @@
class ClusterShellWorker(BaseWorker):
"""It provides a Cumin worker for SSH using the ClusterShell library."""
- def __init__(self, config, target, logger=None):
+ def __init__(self, config, target):
"""Worker ClusterShell constructor.
:Parameters:
according to parent
:py:meth:`cumin.transports.BaseWorker.__init__`.
"""
- super(ClusterShellWorker, self).__init__(config, target, logger)
+ super(ClusterShellWorker, self).__init__(config, target)
self.task = Task.task_self() # Initialize a ClusterShell task
self._handler_instance = None
@@ -55,7 +55,7 @@
# Instantiate handler
# Schedule only the first command for the first batch, the following
ones must be handled by the EventHandler
self._handler_instance = self.handler( # pylint: disable=not-callable
- self.target, self.commands,
success_threshold=self.success_threshold, logger=self.logger)
+ self.target, self.commands,
success_threshold=self.success_threshold)
self.logger.info("Executing commands {commands} on '{num}' hosts:
{hosts}".format(
commands=self.commands, num=len(self.target.hosts),
hosts=self.target.hosts))
@@ -146,7 +146,7 @@
short_command_length = 35
""":py:class:`int`: the length to which a command should be shortened in
various outputs."""
- def __init__(self, target, commands, success_threshold=1.0, logger=None,
**kwargs):
+ def __init__(self, target, commands, success_threshold=1.0, **kwargs):
"""Event handler ClusterShell extension constructor.
If subclasses defines a ``self.pbar_ko`` `tqdm` progress bar, it will
be updated on timeout.
@@ -160,7 +160,7 @@
"""
super(BaseEventHandler, self).__init__()
self.success_threshold = success_threshold
- self.logger = logger or logging.getLogger(__name__)
+ self.logger = logging.getLogger('.'.join((self.__module__,
self.__class__.__name__)))
self.target = target
self.lock = threading.Lock() # Used to update instance variables
coherently from within callbacks
@@ -480,14 +480,14 @@
enough nodes before proceeding with the next one.
"""
- def __init__(self, target, commands, success_threshold=1.0, logger=None,
**kwargs):
+ def __init__(self, target, commands, success_threshold=1.0, **kwargs):
"""Define a custom ClusterShell event handler to execute commands
synchronously.
:Parameters:
according to parent :py:meth:`BaseEventHandler.__init__`.
"""
super(SyncEventHandler, self).__init__(
- target, commands, success_threshold=success_threshold,
logger=logger, **kwargs)
+ target, commands, success_threshold=success_threshold, **kwargs)
self.current_command_index = 0 # Global pointer for the current
command in execution across all nodes
self.start_command()
self.aborted = False
@@ -706,14 +706,14 @@
orchestration between the nodes.
"""
- def __init__(self, target, commands, success_threshold=1.0, logger=None,
**kwargs):
+ def __init__(self, target, commands, success_threshold=1.0, **kwargs):
"""Define a custom ClusterShell event handler to execute commands
asynchronously between nodes.
:Parameters:
according to parent :py:meth:`BaseEventHandler.__init__`.
"""
super(AsyncEventHandler, self).__init__(
- target, commands, success_threshold=success_threshold,
logger=logger, **kwargs)
+ target, commands, success_threshold=success_threshold, **kwargs)
self.pbar_ok = tqdm(desc='PASS', total=self.counters['total'],
leave=True, unit='hosts', dynamic_ncols=True,
bar_format=colorama.Fore.GREEN + self.bar_format,
file=sys.stderr)
--
To view, visit https://gerrit.wikimedia.org/r/386399
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I44eaed95ae4e7d12f16a03029e7c753bbefccb71
Gerrit-PatchSet: 1
Gerrit-Project: operations/software/cumin
Gerrit-Branch: master
Gerrit-Owner: Volans <[email protected]>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
