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

Set up standard logging module #2064

Closed
wants to merge 73 commits into from
Closed

Set up standard logging module #2064

wants to merge 73 commits into from

Conversation

rlerm
Copy link

@rlerm rlerm commented Sep 5, 2021

Initial parts of #2063: Set up the root logger from the logging module. Make py3.log defer to that logger.

The root logger's configuration was built in a way that tries to preserve the existing behavior as much as possble.

rlerm and others added 14 commits August 22, 2021 21:01
This has the advantage of overall less custom code; as well as support
for per-module configuration. This would enable a potential solution for
ultrabug#1479, since in the future
it can allow per-module configuration of log levels.

I expect this to mainly help module creators, allowing them to enable
logging for only their module, while disabling all other messages. It
also is easier to use, since the `logging` module's interface is
generally simpler than `self.py3`.

Here, I've made the decision to keep the message format as close as
possible to the existing log messages.
Instead of checking for the "debug" option, we just log with the DEBUG
level. The root logger's configuration will suppress log levels
as needed.
This has the advantage of overall less custom code; as well as support
for per-module configuration. This would enable a potential solution for
ultrabug#1479, since in the future
it can allow per-module configuration of log levels.

I expect this to mainly help module creators, allowing them to enable
logging for only their module, while disabling all other messages. It
also is easier to use, since the `logging` module's interface is
generally simpler than `self.py3`.

Here, I've made the decision to keep the message format as close as
possible to the existing log messages.
Instead of checking for the "debug" option, we just log with the DEBUG
level. The root logger's configuration will suppress log levels
as needed.
@lgtm-com
Copy link
Contributor

lgtm-com bot commented Sep 5, 2021

This pull request introduces 1 alert when merging ac08112 into cafe0c7 - view on LGTM.com

new alerts:

  • 1 for Unused import

@rlerm rlerm changed the title Logging Set up standard logging module Sep 5, 2021
@ultrabug
Copy link
Owner

ultrabug commented Sep 7, 2021

Thanks @rlerm, this is a very promising first contribution to this project 🏅

When you say "initial parts" I guess you plan on adding things, is that correct? If so, please mark this PR as WIP and tell me when the time is right to review and test it please?

Also, you will want to demonstrate and document what this new logging system brings to the table and how to use it to achieve the goals you expressed in #2063 please

Thanks again

rlerm and others added 9 commits September 18, 2021 16:44
This has the advantage of overall less custom code; as well as support
for per-module configuration. This would enable a potential solution for
ultrabug#1479, since in the future
it can allow per-module configuration of log levels.

I expect this to mainly help module creators, allowing them to enable
logging for only their module, while disabling all other messages. It
also is easier to use, since the `logging` module's interface is
generally simpler than `self.py3`.

Here, I've made the decision to keep the message format as close as
possible to the existing log messages.
Instead of checking for the "debug" option, we just log with the DEBUG
level. The root logger's configuration will suppress log levels
as needed.
ultrabug and others added 22 commits January 31, 2022 19:56
The JSON file will take over the entire configuration; it must fully
specify logging as defined in
https://docs.python.org/3/library/logging.config.html?highlight=logging#logging-config-dictschema
This has the advantage of overall less custom code; as well as support
for per-module configuration. This would enable a potential solution for
ultrabug#1479, since in the future
it can allow per-module configuration of log levels.

I expect this to mainly help module creators, allowing them to enable
logging for only their module, while disabling all other messages. It
also is easier to use, since the `logging` module's interface is
generally simpler than `self.py3`.

Here, I've made the decision to keep the message format as close as
possible to the existing log messages.
Instead of checking for the "debug" option, we just log with the DEBUG
level. The root logger's configuration will suppress log levels
as needed.
This has the advantage of overall less custom code; as well as support
for per-module configuration. This would enable a potential solution for
ultrabug#1479, since in the future
it can allow per-module configuration of log levels.

I expect this to mainly help module creators, allowing them to enable
logging for only their module, while disabling all other messages. It
also is easier to use, since the `logging` module's interface is
generally simpler than `self.py3`.

Here, I've made the decision to keep the message format as close as
possible to the existing log messages.
Instead of checking for the "debug" option, we just log with the DEBUG
level. The root logger's configuration will suppress log levels
as needed.
The Python library defaults to True, but that is a confusing behavior
since it disables the "core" logger, which was created before logging
was set up. Users can still set 'disable_existing_loggers' explicitly in
their configs.

Since most existing modules (who are using self.py3.log) will be logged
by "core", that means that most logs would be suppressed.
Also, use it as the logging example in the docs.
@rlerm
Copy link
Author

rlerm commented Jan 31, 2022

I've done the rebase, can you check it again? Thanks!

@ultrabug
Copy link
Owner

ultrabug commented Feb 1, 2022

@rlerm as I'm trying to figure out how to get the same behavior as the current syslog state using your PR I fail to get a debug log to systemd but I don't get debug logs of the core/py3status if I try to use:

{
    "version": 1,
    "handlers": {
        "syslog": {
            "formatter": "default",
            "class": "logging.handlers.SysLogHandler"
        },
        "file": {
            "class": "logging.handlers.RotatingFileHandler",
            "filename": "/tmp/py3status_log.log",
            "maxBytes": 2048,
            "formatter": "default"
        }
    },
    "formatters": {
        "default": {
            "validate": false,
            "format":"%(asctime)s %(levelname)s %(module)s %(message)s",
            "datefmt":"%Y-%m-%d %H:%M:%S"
        }
    },
    "loggers": {
        "root": {"handlers": ["syslog"], "level": "DEBUG"},
        "core": {"handlers": ["syslog"], "level": "DEBUG"},
        "xrandr": {"handlers": ["file"], "level": "DEBUG"}
    }
}

what I am missing please?

@rlerm
Copy link
Author

rlerm commented Mar 10, 2022

You were missing https://stackoverflow.com/a/3969772/340862: the syslog handler requires an "address" parameter (we are setting it correctly in the default setting in core.py.

By adding the same parameter to the json config, I am able to get the logs for core:

    "handlers": {
        "syslog": {
            "formatter": "default",
            "class": "logging.handlers.SysLogHandler",
            "address": "/dev/log"
        },
(...)

@lgtm-com
Copy link
Contributor

lgtm-com bot commented Oct 2, 2022

This pull request introduces 1 alert when merging 0049671 into afeab68 - view on LGTM.com

new alerts:

  • 1 for Variable defined multiple times

@ultrabug
Copy link
Owner

closing, I never got to the bottom of it and I apologize for this

recent updates would require work from you

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants