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

Implement module scoped logging #361

Merged
merged 5 commits into from
Apr 24, 2024
Merged

Implement module scoped logging #361

merged 5 commits into from
Apr 24, 2024

Conversation

MaxDall
Copy link
Collaborator

@MaxDall MaxDall commented Feb 17, 2024

Since #357 rewrites a huge part of the core pipeline, this PR is based on the redesign and should be merged afterwards.

This removes the central logging solution of basic_logger and implements module-scoped logging. Every module utilizing logging must now have its own logger called __module_logger__.

from fundus.logging import create_logger

__module_logger__ = create_logger(__name__)

One can set a global log level

import logging
from fundus.logging import set_log_level

set_log_level(logging.DEBUG)

effecting only loggers created with create_logger, or setting the log level of individual module-scoped loggers by importing them.

import logging
from fundus.scraping.html import __module_logger__

__module_logger__.setLevel(logging.DEBUG)

@MaxDall MaxDall added the DON'T MERGE Current PR is based on another PR. When used, indicate in title with [Based on #...] label Feb 17, 2024
@MaxDall MaxDall changed the base branch from master to unbatch-fundus February 17, 2024 16:42
@MaxDall MaxDall added feature Have an idea on how to improve the code base? Come forward and let us know. enhancement Improvements upon existing features. and removed feature Have an idea on how to improve the code base? Come forward and let us know. labels Feb 17, 2024
Base automatically changed from unbatch-fundus to master April 18, 2024 10:28
src/fundus/logging/__init__.py Outdated Show resolved Hide resolved
src/fundus/parser/data.py Outdated Show resolved Hide resolved
Comment on lines 12 to 22
def create_logger(name: str) -> logging.Logger:
logger = logging.getLogger(name)
logger.setLevel(logging.ERROR)
logger.addHandler(_stream_handler)
_loggers.append(logger)
return logger


def set_log_level(level: int):
for logger in _loggers:
logger.setLevel(level)
Copy link
Collaborator

Choose a reason for hiding this comment

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

This definitely works. But I wondered if such a setup is possible where each module can just call logger.getLogger(__name__) to get their module logger, just like in the python logging guide. I failed to get this to work but wanted to give the idea here. Maybe you tried this as well?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Good idea. The only problem is see is how we set the log level for all Fundus logger at once? Setting them individually wouldn't be a problem, but I have no solution for how we would set the whole library to DEBUG without keeping track of the loggers.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yes, that was also a problem I encountered. I found this comment but haven't been able to get this working. I would merge this for now as is and maybe try later.

@dobbersc dobbersc removed the DON'T MERGE Current PR is based on another PR. When used, indicate in title with [Based on #...] label Apr 20, 2024
# Conflicts:
#	scripts/generate_parser_test_files.py
#	src/fundus/parser/data.py
#	src/fundus/scraping/crawler.py
#	src/fundus/scraping/scraper.py
#	src/fundus/scraping/session.py
dobbersc
dobbersc previously approved these changes Apr 22, 2024
@MaxDall MaxDall merged commit c931e2e into master Apr 24, 2024
4 checks passed
@MaxDall MaxDall deleted the module_logging branch April 24, 2024 15:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Improvements upon existing features.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants