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

Refresh all filehandlers on named logger. #33

Merged
merged 1 commit into from
May 1, 2024

Conversation

JoeZiminski
Copy link
Member

@JoeZiminski JoeZiminski commented May 1, 2024

This PR resets the handlers on a named logger to empty so that repeated calls to fancylog respect the most recent calls arguments. This also adds a test to check the handlers are set as expected. Currently handlers are not removed from the
root logger as I'm not sure if it would have unexpected consequences. It does mean that this kind of example below can happen:

import logging
import fancylog

random_unused_filepath = r"random"

fancylog.start_logging(
    random_unused_filepath, fancylog, logger_name=None, log_to_console=True, log_to_file=False
)

logging.debug("written to console as `log_to_console=True`")

fancylog.start_logging(
    random_unused_filepath, fancylog, logger_name=None, log_to_console=False, log_to_file=False
)

logging.debug("still written to console because filehandler persists on "
              "the root logger even though `log_to_console=False`.")

But I think generally people will call fancylog.start_logging only once so not much of a problem, but makes sense to set for named loggers. Maybe in the docs in future the recommended way to use fancylog can be with a named logger as the behaviour is more predictable.

@adamltyson
Copy link
Member

Maybe in the docs in future the recommended way to use fancylog can be with a named logger as the behaviour is more predictable.

👍

@adamltyson adamltyson merged commit be62d33 into master May 1, 2024
19 checks passed
@adamltyson adamltyson deleted the clear_named_logger_handlers branch May 1, 2024 16:48
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