New issue
Advanced search Search tips

Issue 735333 link

Starred by 1 user

Issue metadata

Status: Available
Owner: ----
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

Logging functions in blinkpy/style/checker.py duplicate much of blinkpy/common/system/log_utils.py

Project Member Reported by tansell@chromium.org, Jun 21 2017

Issue description

The logging functions below, found in webkitpy/style/checker.py
----------------------
def _create_log_handlers(stream, no_time):
    """Create and return a default list of logging.Handler instances.

    Format WARNING messages and above to display the logging level, and
    messages strictly below WARNING not to display it.

    Args:
      stream: See the configure_logging() docstring.
      no_time: See the configure_logging() docstring.
    """
    # Handles logging.WARNING and above.
    error_handler = logging.StreamHandler(stream)
    error_handler.setLevel(logging.WARNING)
    if no_time:
        formatter = logging.Formatter('%(levelname)s: %(message)s')
    else:
        formatter = logging.Formatter('%(asctime)s %(levelname)s: %(message)s')
    error_handler.setFormatter(formatter)

    # Create a logging.Filter instance that only accepts messages
    # below WARNING (i.e. filters out anything WARNING or above).
    non_error_filter = logging.Filter()
    # The filter method accepts a logging.LogRecord instance.
    non_error_filter.filter = lambda record: record.levelno < logging.WARNING

    non_error_handler = logging.StreamHandler(stream)
    non_error_handler.addFilter(non_error_filter)
    formatter = logging.Formatter('%(message)s')
    non_error_handler.setFormatter(formatter)

    return [error_handler, non_error_handler]


def _create_debug_log_handlers(stream, no_time):
    """Create and return a list of logging.Handler instances for debugging.

    Args:
      stream: See the configure_logging() docstring.
      no_time: See the configure_logging() docstring.
    """
    handler = logging.StreamHandler(stream)
    formatter = logging.Formatter('%(name)s: %(levelname)-8s %(message)s')
    handler.setFormatter(formatter)

    return [handler]


def configure_logging(stream, logger=None, is_verbose=False):
    """Configure logging, and return the list of handlers added.

    Returns:
      A list of references to the logging handlers added to the root
      logger.  This allows the caller to later remove the handlers
      using logger.removeHandler.  This is useful primarily during unit
      testing where the caller may want to configure logging temporarily
      and then undo the configuring.

    Args:
      stream: A file-like object to which to log.  The stream must
              define an "encoding" data attribute, or else logging
              raises an error.
      logger: A logging.logger instance to configure.  This parameter
              should be used only in unit tests.  Defaults to the
              root logger.
      is_verbose: A boolean value of whether logging should be verbose.
    """
    # If the stream does not define an "encoding" data attribute, the
    # logging module can throw an error like the following:
    #
    # Traceback (most recent call last):
    #   File "/System/Library/Frameworks/Python.framework/Versions/2.6/...
    #         lib/python2.6/logging/__init__.py", line 761, in emit
    #     self.stream.write(fs % msg.encode(self.stream.encoding))
    # LookupError: unknown encoding: unknown
    if logger is None:
        logger = logging.getLogger()

    if is_verbose:
        logging_level = logging.DEBUG
        handlers = _create_debug_log_handlers(stream, no_time=no_time)
    else:
        logging_level = logging.INFO
        handlers = _create_log_handlers(stream, no_time=no_time)

    handlers = _configure_logging(logging_level=logging_level, logger=logger,
                                  handlers=handlers)

    return handlers
----------------------

Seem very similar to the logging functions in webkitpy/common/system/log_utils.py
----------------------
def _default_handlers(stream, logging_level, no_time):
    """Return a list of the default logging handlers to use.

    Args:
      stream: See the configure_logging() docstring.
      no_time: See the configure_logging() docstring.
    """
    # Create the filter.
    def should_log(record):
        """Return whether a logging.LogRecord should be logged."""
        if record.name.startswith('webkitpy.thirdparty'):
            return False
        return True

    logging_filter = logging.Filter()
    logging_filter.filter = should_log

    # Create the handler.
    handler = logging.StreamHandler(stream)
    if no_time:
        prefix = ''
    else:
        prefix = '%(asctime)s - '

    if logging_level == logging.DEBUG:
        formatter = logging.Formatter(prefix + '%(name)s: [%(levelname)s] %(message)s')
    else:
        formatter = logging.Formatter(prefix + '%(message)s')

    handler.setFormatter(formatter)
    handler.addFilter(logging_filter)

    return [handler]


def configure_logging(logging_level=None, logger=None, stream=None,
                      handlers=None, no_time=False):
    """Configure logging for standard purposes.

    Returns:
      A list of references to the logging handlers added to the root
      logger.  This allows the caller to later remove the handlers
      using logger.removeHandler.  This is useful primarily during unit
      testing where the caller may want to configure logging temporarily
      and then undo the configuring.

    Args:
      logging_level: The minimum logging level to log.  Defaults to
                     logging.INFO.
      logger: A logging.logger instance to configure.  This parameter
              should be used only in unit tests.  Defaults to the
              root logger.
      stream: A file-like object to which to log used in creating the default
              handlers.  The stream must define an "encoding" data attribute,
              or else logging raises an error.  Defaults to sys.stderr.
      handlers: A list of logging.Handler instances to add to the logger
                being configured.  If this parameter is provided, then the
                stream parameter is not used.
    """
    # If the stream does not define an "encoding" data attribute, the
    # logging module can throw an error like the following:
    #
    # Traceback (most recent call last):
    #   File "/System/Library/Frameworks/Python.framework/Versions/2.6/...
    #         lib/python2.6/logging/__init__.py", line 761, in emit
    #     self.stream.write(fs % msg.encode(self.stream.encoding))
    # LookupError: unknown encoding: unknown
    if logging_level is None:
        logging_level = logging.INFO
    if logger is None:
        logger = logging.getLogger()
    if stream is None:
        stream = sys.stderr
    if handlers is None:
        handlers = _default_handlers(stream, logging_level, no_time)

    logger.setLevel(logging_level)

    for handler in handlers:
        logger.addHandler(handler)

    _log.debug('Debug logging enabled.')

    return handlers
----------------------

It seems like these should be merged in some way?

Having these duplication functions confused me a lot initially when trying to add the timing information stuff. Even lots of the tests seem to be duplicate.

 
Labels: Hotlist-CodeHealth
Status: Available (was: Untriaged)
Summary: Logging functions in webkitpy/style/checker.py duplicate much of webkitpy/common/system/log_utils.py (was: logging functions in webkitpy/style/checker.py seems to duplicate much of webkitpy/common/system/log_utils.py)
Project Member

Comment 2 by sheriffbot@chromium.org, Aug 10

Labels: Hotlist-Recharge-Cold
Status: Untriaged (was: Available)
This issue has been Available for over a year. If it's no longer important or seems unlikely to be fixed, please consider closing it out. If it is important, please re-triage the issue.

Sorry for the inconvenience if the bug really should have been left as Available.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Status: Available (was: Untriaged)
Summary: Logging functions in blinkpy/style/checker.py duplicate much of blinkpy/common/system/log_utils.py (was: Logging functions in webkitpy/style/checker.py duplicate much of webkitpy/common/system/log_utils.py)

Sign in to add a comment