Skip to content

bpo-34844: logging.Formatter enhancement - Ensure styles and fmt matches in logging.Formatter #9703

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.

Sign up for GitHub

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

Merged
vsajip merged 21 commits into from
Oct 15, 2018
Merged

bpo-34844: logging.Formatter enhancement - Ensure styles and fmt matches in logging.Formatter #9703

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
66bbddf
bpo-34844
BNMetrics Oct 4, 2018
0eaaf25
added news entry
BNMetrics Oct 4, 2018
f4f6a60
suggested changed before regex fix
BNMetrics Oct 5, 2018
d51e186
regex changes
BNMetrics Oct 6, 2018
cc2cfbd
commit
BNMetrics Oct 6, 2018
7c24e46
rewrote regex
BNMetrics Oct 6, 2018
abd868d
changes as requested
BNMetrics Oct 7, 2018
c1359bb
changes as requested
BNMetrics Oct 7, 2018
921ba33
added doc and docstring, added some of the original tests back for $ …
BNMetrics Oct 8, 2018
8484f29
changed validate method for StrFormatStyle
BNMetrics Oct 9, 2018
4c35216
changed on StrFormatStyle class
BNMetrics Oct 10, 2018
6eee298
commit
BNMetrics Oct 12, 2018
aab0c46
removed redundant debugging print
BNMetrics Oct 13, 2018
59879e9
added options to logging.config.dictConfig to allow passing in valida…
BNMetrics Oct 13, 2018
43c0b34
added in documentation for the validate key in logging.config.dictConfig
BNMetrics Oct 13, 2018
908c3ec
removing blank space
BNMetrics Oct 14, 2018
d73ab3a
rewrote logging.config.DictConfigurator.configure_formatter logic, si…
BNMetrics Oct 14, 2018
ff05742
document typo change on logging.rst
BNMetrics Oct 14, 2018
2664f85
reformatted logging.config.rst
BNMetrics Oct 14, 2018
74ca6a2
simplified the logic in logging.config.DictConfig.configure_formatter
BNMetrics Oct 15, 2018
de1243f
fixed document due to failure
BNMetrics Oct 15, 2018
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
4 changes: 2 additions & 2 deletions Doc/library/logging.config.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -227,9 +227,9 @@ otherwise, the context is used to determine what to instantiate.
:class:`~logging.Formatter` instance.

.. versionchanged:: 3.8
a ``validate`` key (with defaults of ``True``) can be added into
a ``validate`` key (with default of ``True``) can be added into
the ``formatters`` section of the configuring dict, this is to
validate the correct format.
validate the format.

* *filters* - the corresponding value will be a dict in which each key
is a filter id and each value is a dict describing how to configure
Expand Down
32 changes: 15 additions & 17 deletions Lib/logging/config.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,6 @@
import sys
import threading
import traceback
from inspect import signature

from socketserver import ThreadingTCPServer, StreamRequestHandler

Expand Down Expand Up @@ -667,28 +666,27 @@ def configure_formatter(self, config):
dfmt = config.get('datefmt', None)
style = config.get('style', '%')
cname = config.get('class', None)
validate = config.get('validate', True)

if not cname:
c = logging.Formatter
result = c(fmt, dfmt, style, validate)
else:
c = _resolve(cname)
if self._has_validate_param(config, c):
result = c(fmt, dfmt, style, validate)
else:
result = c(fmt, dfmt, style)
return result

def _has_validate_param(self, config, formatter_callable):
"""Check if formatter callable has validate parameter"""
params = signature(formatter_callable).parameters
if not isinstance(formatter_callable, logging.Formatter) or \
not params.get('validate') or params.get("kwargs"):
if config.get('validate'):
raise TypeError('%r has not validate in parameters' % formatter_callable)
# Identifying if we need to pass in the validate parameter to the formatter
# - The default in logging.Formatter is True, so we won't need to pass in
# if "validate" key is not in the config
use_validate = False

@vsajip vsajip Oct 15, 2018

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

OK, much simpler than my overthinking! But that's not right, because if the user specified False in the config, then config.get('validate') would return False, so use_validate would not be set to True and the user's False would never be passed to the call.

IMO you could lose the use_validate altogether and simplify even further to:

if 'validate' in config:  # if user hasn't mentioned it, the default will be fine
    result = c(fmt, dfmt, style, config['validate'])
else:
    result = c(fmt, dfmt, style)

Notice my stubborn sticking to single quotes 😉

if config.get("validate"):
use_validate = True

# A TypeError would be raised if "validate" key is passed in with a formatter callable
# that does not accept "validate" as a parameter
if use_validate:
result = c(fmt, dfmt, style, config["validate"])
else:
result = c(fmt, dfmt, style)

return False
return True
return result

def configure_filter(self, config):
"""Configure a filter from a dictionary."""
Expand Down