-
Notifications
You must be signed in to change notification settings - Fork 1
Feat/custome handler #16
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -40,12 +40,25 @@ def get_logger(self, name: str) -> logging.Logger: | |||||||||||||
| # Add handlers if they don't exist | ||||||||||||||
| if not logger.handlers: | ||||||||||||||
| self._setup_handlers(logger) | ||||||||||||||
| self._setup_custom_handlers(logger) | ||||||||||||||
|
|
||||||||||||||
| return logger | ||||||||||||||
|
|
||||||||||||||
| def _setup_custom_handlers(self, logger: logging.Logger): | ||||||||||||||
| if not hasattr(self.settings, "custom_handlers"): | ||||||||||||||
| return | ||||||||||||||
|
|
||||||||||||||
| for handler in self.settings.custom_handlers: | ||||||||||||||
| if not isinstance(handler, logging.Handler): | ||||||||||||||
|
||||||||||||||
| if not isinstance(handler, logging.Handler): | |
| if not isinstance(handler, logging.Handler): | |
| logging.getLogger(__name__).warning( | |
| f"Skipping invalid custom handler: {type(handler).__name__}. " | |
| "Expected logging.Handler instance." | |
| ) |
Copilot
AI
Nov 27, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Custom handlers always receive a formatter based on use_console_colored_formatter, which may not be appropriate for all custom handlers. For example, if a user adds a file-based custom handler, they might want use_file_colored_formatter instead, or a completely different formatter.
Consider one of these approaches:
- Allow users to pass handlers that already have formatters set, and only apply a formatter if the handler doesn't already have one
- Add a configuration option to specify which formatter setting applies to custom handlers
- Document this behavior clearly so users know custom handlers will use the console formatter settings
Copilot
AI
Nov 27, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The new custom_handlers feature lacks test coverage. Since the repository has comprehensive test coverage for other logging features (see test/logger_test.py), this new functionality should also be tested. Consider adding tests that verify:
- Custom handlers are properly added to the logger
- Custom handlers receive the correct formatter
- Custom handlers get the appropriate filters (tracing, application level)
- Invalid handlers are handled gracefully
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -4,7 +4,7 @@ | |||||
| from typing import Literal, Optional | ||||||
|
|
||||||
| import click | ||||||
| from pydantic import BaseModel | ||||||
| from pydantic import BaseModel, Field | ||||||
|
|
||||||
|
|
||||||
| class LoggingSettings(BaseModel): | ||||||
|
|
@@ -26,6 +26,7 @@ class LoggingSettings(BaseModel): | |||||
| use_syslog_colored_formatter: bool = False | ||||||
| use_file_colored_formatter: bool = False | ||||||
| show_process_id: bool = False | ||||||
| custom_handlers: list = Field(default_factory=list) | ||||||
|
||||||
| custom_handlers: list = Field(default_factory=list) | |
| custom_handlers: list[logging.Handler] = Field(default_factory=list) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The
hasattrcheck on line 48 is unnecessary sincecustom_handlersis now a defined field in theLoggingSettingsPydantic model with a default value. The attribute will always exist, so this check can be removed.The method can be simplified to: