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.

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

Support Dictionary Config setting. #1602

Merged
merged 5 commits into from
Oct 30, 2017
Merged

Support Dictionary Config setting. #1602

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
e6de1a0
Issue 1087: support dictConfig logging configuration
Sep 3, 2015
9d7c857
The logconfig_dict setting is not available on 2.6
MatMoore Sep 5, 2015
c5ae962
Add CLI option for dictionary log config
tilgovi Oct 30, 2017
a7d4491
Fix up documentation for dictConfig
tilgovi Oct 30, 2017
783c9be
Warn on Python < 2.7 for dictConfig
tilgovi Oct 30, 2017
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
18 changes: 18 additions & 0 deletions gunicorn/config.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1309,6 +1309,24 @@ class LogConfig(Setting):
"""


class LogConfigDict(Setting):
name = "logconfig_dict"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do we need to add cli = ["--log-config-dict"]?

section = "Logging"
cli = ["--log-config-dict"]
Copy link

Choose a reason for hiding this comment

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

This doesn't seem to work, as there doesn't seem to be any conversion from the string supplied by the shell in some ARGV[x] to an actual dict:

bash-3.2$ env/bin/gunicorn --version
env/bin/gunicorn --version
gunicorn (version 19.9.0)
bash-3.2$ env/bin/gunicorn --log-config-dict {} 'flask.main:app'
env/bin/gunicorn --log-config-dict {} 'flask.main:app'

Error: Value is not a dictionary: {}

Copy link
Collaborator

Choose a reason for hiding this comment

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

Right, maybe cli should have been left off this option until someone JSON-parses it. Can you use config file?

Also https://stackoverflow.com/a/18609361/452210 re argparse capabilities in this area.

Copy link

Choose a reason for hiding this comment

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

I think we can extend our config we're passing via --config=python:cariumlib.djangolib.gunicorn_config
It's funny, I tried hacking type=json.loads in, but couldn't get it to work.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe you could convert your PR comment into a new issue along the lines of advertised CLI option --log-config-dict cannot work.

Copy link

Choose a reason for hiding this comment

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

@javabrett Done: #1909

validator = validate_dict
default = {}
Copy link
Collaborator

Choose a reason for hiding this comment

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

This might be a little bit misleading since we use glogging.CONFIG_DEFAULTS as default. Should we document this or keep it as implementation detail?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Maybe we should set the default as None? Would that be more "unset" / "undefined" / "default" to people?

Maybe it doesn't matter much either way.

desc = """\
The log config dictionary to use, using the standard Python
logging module's dictionary configuration format. This option
takes precedence over the :ref:`logconfig` option, which uses the
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is a minor issue, but I'd say let's deprecate --log-config documentation-only and tell users to switch to --log-config-dict.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

If you can explain a bit more, I will do a PR. Or I will review a PR. Thanks!

older file configuration format.

Format: https://docs.python.org/3/library/logging.config.html#logging.config.dictConfig

.. versionadded:: 19.8
"""


class SyslogTo(Setting):
name = "syslog_addr"
section = "Logging"
Expand Down
23 changes: 22 additions & 1 deletion gunicorn/glogging.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,11 @@
import logging
logging.Logger.manager.emittedNoHandlerWarning = 1
from logging.config import fileConfig
try:
from logging.config import dictConfig
except ImportError:
# python 2.6
dictConfig = None
import os
import socket
import sys
Expand Down Expand Up @@ -226,7 +231,23 @@ def setup(self, cfg):
self.access_log, cfg, self.syslog_fmt, "access"
)

if cfg.logconfig:
if dictConfig is None and cfg.logconfig_dict:
util.warn("Dictionary-based log configuration requires "
"Python 2.7 or above.")

if dictConfig and cfg.logconfig_dict:
config = CONFIG_DEFAULTS.copy()
config.update(cfg.logconfig_dict)
try:
dictConfig(config)
except (
AttributeError,
ImportError,
ValueError,
TypeError
) as exc:
raise RuntimeError(str(exc))
elif cfg.logconfig:
if os.path.exists(cfg.logconfig):
defaults = CONFIG_DEFAULTS.copy()
defaults['__file__'] = cfg.logconfig
Expand Down