Pull requests

#169 Merged
Repository
davidlindquist davidlindquist
Branch
default
Repository
logilab logilab
Branch
default

Add new warning: logging-format-interpolation

Author
  1. David Lindquist
Reviewers
Description

Emit warning for .format() interpolation within logging function calls.

Comments (7)

    1. Claudiu Popa

      Because the message will be evaluated (interpolated), even if the message will not be logged, due to a different warning level set. For instance, the following is always better than using .format.

      log.info("test %s %s", a, b) # interpolated only when the message will be emitted
      

      Also, it's not an "enforcement", but a style guide or an improvement. Not every Pylint message can be considered an error or an warning. Well, we should add an Information category though. Such messages can then be moved in that section.

  1. Pedro Algarvio

    Some more arguments to why I think this warning should not be emitted to log strings containing {0} formatting...

    >>> import logging
    >>> import sys
    >>> logging.basicConfig(level=logging.DEBUG, stream=sys.stdout)
    >>> log = logging.getLogger()
    >>> log.warn('foo')
    WARNING:root:foo
    >>> log.warn('%s', 'foo')
    WARNING:root:foo
    >>> log.warn('{0}', 'foo')
    Traceback (most recent call last):
      File "/home/vampas/.dotfiles/.ext/pyenv/versions/2.6.9/lib/python2.6/logging/__init__.py", line 776, in emit
        msg = self.format(record)
      File "/home/vampas/.dotfiles/.ext/pyenv/versions/2.6.9/lib/python2.6/logging/__init__.py", line 654, in format
        return fmt.format(record)
      File "/home/vampas/.dotfiles/.ext/pyenv/versions/2.6.9/lib/python2.6/logging/__init__.py", line 436, in format
        record.message = record.getMessage()
      File "/home/vampas/.dotfiles/.ext/pyenv/versions/2.6.9/lib/python2.6/logging/__init__.py", line 306, in getMessage
        msg = msg % self.args
    TypeError: not all arguments converted during string formatting
    >>> sys.version_info
    (2, 6, 9, 'final', 0)
    >>> 
    

    That's for Python 2.6, but go figure....

    >>> import logging
    >>> import sys
    >>> logging.basicConfig(level=logging.DEBUG, stream=sys.stdout)
    >>> log = logging.getLogger()
    >>> log.warn('foo')
    WARNING:root:foo
    >>> log.warn('%s', 'foo')
    WARNING:root:foo
    >>> log.warn('{0}', 'foo')
    Traceback (most recent call last):
      File "/home/vampas/.dotfiles/.ext/pyenv/versions/2.7.8/lib/python2.7/logging/__init__.py", line 859, in emit
        msg = self.format(record)
      File "/home/vampas/.dotfiles/.ext/pyenv/versions/2.7.8/lib/python2.7/logging/__init__.py", line 732, in format
        return fmt.format(record)
      File "/home/vampas/.dotfiles/.ext/pyenv/versions/2.7.8/lib/python2.7/logging/__init__.py", line 471, in format
        record.message = record.getMessage()
      File "/home/vampas/.dotfiles/.ext/pyenv/versions/2.7.8/lib/python2.7/logging/__init__.py", line 335, in getMessage
        msg = msg % self.args
    TypeError: not all arguments converted during string formatting
    Logged from file <stdin>, line 1
    >>> sys.version_info
    sys.version_info(major=2, minor=7, micro=8, releaselevel='final', serial=0)
    >>> 
    

    Python 2.7 also doesn't support that, so, why would PyLint thrown a warning in this case?

  2. Pedro Algarvio

    Well I see your point. And since formatting is not supported by the logging module it does make sense. Our project adopted string formating and it wouldn't make much sense to advise substitution just for logging. It would bring confusion to contributors. We'll just disable this check.