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.

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

logging: add format and datefmt to allow configuring the log output from cfg file #1512

Merged
merged 1 commit into from
Jun 10, 2019

Conversation

ddstreet
Copy link

logging currently uses the default msg format and date format; add cfg file parameters to allow users to specify what format they would like for both msgs and timestamps.

Copy link
Member

@dgw dgw left a comment

Choose a reason for hiding this comment

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

I'm 👍 on making logging more configurable, but this patch has at least one error that must be fixed before we can proceed. See the line notes for more detailed feedback.

sopel/logger.py Outdated Show resolved Hide resolved
sopel/logger.py Outdated Show resolved Hide resolved
@dgw dgw added the Feature label Mar 22, 2019
@dgw dgw added this to the 7.0.0 milestone Mar 22, 2019
@ddstreet
Copy link
Author

level is no longer defined after the changes above. This failed the linter.

oops! will fix that

I'd rather not shortcut-reference the core config section like this

will change this also.

@ddstreet ddstreet force-pushed the loggingfmt branch 2 times, most recently from a652dfb to db74a99 Compare March 22, 2019 21:28
Copy link
Contributor

@Exirel Exirel left a comment

Choose a reason for hiding this comment

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

I think you can use intermediate variables more often, but otherwise: looks very good.

sopel/logger.py Outdated Show resolved Hide resolved
@ddstreet
Copy link
Author

updated as suggested; please let me know if you have any add'l requests/suggestions. thnx!

…rom cfg file

also add IRC channel-specific logging params to set level, format, and datefmt
for IRC channel logs, different from the local logs.  This can be useful when
the format of the IRC logs should be different than the format of the local logs.
This is also quite useful when the local log level is set to DEBUG, as logging
a huge volume of DEBUG logs to IRC is rarely useful.
@ddstreet
Copy link
Author

(pushed minor indentation correction)

@Exirel
Copy link
Contributor

Exirel commented Mar 27, 2019

LGTM 👍 :shipit: 🦄

@Exirel
Copy link
Contributor

Exirel commented May 1, 2019

This should fix #1097 or at least it should allow users to configure logging in a way that fixes it for them.

@Exirel
Copy link
Contributor

Exirel commented May 8, 2019

@dgw another quick win PR for you to merge.

@Exirel
Copy link
Contributor

Exirel commented May 11, 2019

@dgw I was looking at logging & the output redirect we use for Sopel, and I want to do some work around that. This PR would bring some very interesting feature I'd really like to reuse as soon as possible. If you didn't pick yet your next PR to merge, here is a very good candidate!

Copy link
Member

@dgw dgw left a comment

Choose a reason for hiding this comment

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

Seems straightforward enough to me. Let's get this onto the shortlist of "ready to merge" PRs!

@Exirel Exirel mentioned this pull request May 31, 2019
7 tasks
@dgw dgw merged commit b34f85a into sopel-irc:master Jun 10, 2019
@Exirel Exirel mentioned this pull request Jul 6, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants