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

feat: Provide Module-specific Logger #904

Open
wants to merge 3 commits into
base: develop
Choose a base branch
from

Conversation

phorward
Copy link
Member

Introduces log-member to Modules, so logs can be filtered by module and are also printed so to the local formatter, using 'Module: message'. No need to import logging to all your ViUR modules anymore, just use self.log.

Introduces `log`-member to Modules, so logs can be filtered by module and are also printed so to the local formatter, using 'Module: message'. No need to import logging to all your ViUR modules anymore, just use `self.log`.
@phorward phorward added the feature New feature or request label Oct 11, 2023
@phorward phorward changed the title feat: Provide Module-specific Logger feat: Provide Module-specific Logger Oct 11, 2023
@phorward phorward added this to the ViUR-core v3.6 milestone Oct 11, 2023
@phorward phorward added the viur-meeting Issues to discuss in the next ViUR meeting label Oct 11, 2023
@@ -110,6 +110,10 @@ def format(self, record: logging.LogRecord) -> str:

record.pathname = pathname

# Show logger-name before message when not "root" logger
if record.name != "root":
Copy link
Member

Choose a reason for hiding this comment

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

Sorry, but this condition is not good. This effects all other loggers. Not only the module-loggers.

The logging in the viur-core is general kinda hackish. Using the root logger is a bad practise, because usually all loggers of other packages propagates to it. Which results in workaround like this.

I have long wanted to rework the logging and work with sub loggers... Maybe this would be a good time to combine this. Give me a bit of time to evaluate and get this done.

Copy link
Member Author

Choose a reason for hiding this comment

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

Ok, let's put this thing on hold. You can also create a follow-up PR based on this, if it's easier.

Copy link
Member

Choose a reason for hiding this comment

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

I am working on #1080

@phorward phorward added Priority: Low This issue can be considered with enough idle time. waiting-for-changes Waiting for changes/rework from the author and removed viur-meeting Issues to discuss in the next ViUR meeting labels Oct 13, 2023
@sveneberth sveneberth self-assigned this Dec 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature New feature or request Priority: Low This issue can be considered with enough idle time. waiting-for-changes Waiting for changes/rework from the author
Projects
Status: Todo
Development

Successfully merging this pull request may close these issues.

None yet

2 participants