journal: add extra parameter to JournalHandler constructor #52
Conversation
systemd/journal.py Outdated
| super(JournalHandler, self).__init__(level) | ||
| | ||
| if isinstance(extra, dict): | ||
| kwargs = extra |
There was a problem hiding this comment.
The patch looks good, with the exception of this part. extra completely overrides kwargs. I think it would be better to simply look at both:
for d in (extra, kwargs): for name in d: if not _valid_field_name(name): ... ...There was a problem hiding this comment.
I see.
I updated to look at both by updating kwargs.
https://github.com/systemd/python-systemd/pull/52/files#diff-fbdb822e1b0f59f5302395f64a7fda54R560
Added test: https://github.com/systemd/python-systemd/pull/52/files#diff-900add079f3a059ccbc90179aeec1511R113
This change enables to add extra fields to JournalHandler in a configuration file loaded by `logging.config.fileConfig`. ------ class=systemd.journal.JournalHandler args=(INFO, {'SYSLOG_IDENTIFIER': 'my-cool-app'}) 
| Merged as 5dd8afd. When merging this, I realized that the patch would break backwards compatibility by inserting a new positional argument. I tweaked your patch to add a new constructor classmethod instead. I hope that's OK. Thank you for the patch and sorry for the delay. |
2 participants
Hello,
logging.config.fileConfigsupports onlyargswhich is a list of positional arguments.https://docs.python.org/3/library/logging.config.html#configuration-file-format
But
JournalHandlercurrently accepts only keyword arguments.This change enables to add extra fields to JournalHandler in a configuration file.