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.py: CPython-compatible logging improvements. #806

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

ned-pcs
Copy link

@ned-pcs ned-pcs commented Feb 15, 2024

This PR allows for logging handlers to be added to the root logger and then used by non-root loggers that don't have their own handlers.

It replaces #750 (in which I forgot a period at the end of a commit message title).

It also adds the (CPython-compatible) handlers argument to logging.basicConfig() to ease this initialization:

    import logging
    sh = logging.StreamHandler()
    fh = logging.FileHandler("my.log", mode="a")
    logging.basicConfig(handlers=[sh, fh])

    root_logger = logging.getLogger() # uses sh and fh
    another_logger = logging.getLogger("another") # inherits handlers

It also adds the Logger.removeHandler() method and avoids repeated handler addition.

It also adds the flush() method to StreamHandler and its subclasses.

It also correctly calls the superclass constructor from the StreamHandler constructor and uses a default formatter if a Handler has none set (as in PR #710).

@ned-pcs
Copy link
Author

ned-pcs commented Feb 15, 2024

build failure is due to an unrelated manifest.py issue.

@projectgus
Copy link
Contributor

@ned-pcs If you rebase on master then the build should be fixed now.

@ned-pcs ned-pcs force-pushed the ned-pcs/logging-handler-fixes branch from 72003d1 to cd5bfd4 Compare February 20, 2024 00:00
self.stream = _stream if stream is None else stream
self.terminator = "\n"

def close(self):
def flush(self):
Copy link
Member

Choose a reason for hiding this comment

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

Should there still be a close() method which just calls self.flush()?

Copy link
Author

@ned-pcs ned-pcs Mar 19, 2024

Choose a reason for hiding this comment

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

Good question. According to the Cpython docs, Cpython does not have a StreamHandler.close() method, and so the inherited close() method doesn't do any output. I originally had a close() method that called flush() but removed it after looking at the Cpython documentation.

This commit allows for logging handlers to be added to the root logger
and then used by non-root loggers that don't have their own handlers.

It also adds the (CPython-compatible) handlers argument
to logging.basicConfig() to ease this initialization:

    import logging
    sh = logging.StreamHandler()
    fh = logging.FileHandler("my.log", mode="a")
    logging.basicConfig(handlers=[sh, fh])

    root_logger = logging.getLogger() # uses sh and fh
    another_logger = logging.getLogger("another") # inherits handlers


It also adds the Logger.removeHandler() method and avoids repeated handler
addition.

It also adds the flush() method to StreamHandler and its subclasses.

It also correctly calls the superclass constructor from the StreamHandler
constructor and uses a default formatter if a Handler has none
set (as in PR micropython#710).

Signed-off-by: Ned Konz <[email protected]>
@ned-pcs ned-pcs force-pushed the ned-pcs/logging-handler-fixes branch from cd5bfd4 to 94a38ab Compare March 21, 2024 18:39
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.

3 participants