Logging init config logic minor modifications #48
Conversation
…ion.py/__init__.py
WalkthroughThe changes adjust the logging configuration in Changes
Sequence Diagram(s)sequenceDiagram
participant App as Application
participant LogConfig as log_config.yaml
participant Logging as Logging Library
Note over App: Start logging setup
App->>LogConfig: Attempt to load configuration
alt Configuration loaded successfully
App->>Logging: logging.getLogger(__name__)
App->>Logging: logger.warning("Logging setup complete")
else Exception occurs
Logging-->>App: Exception raised
App->>Logging: logging.basicConfig(...)
App->>Logging: logging.getLogger(__name__)
App->>Logging: logger.exception("Exception in loading logging config")
end
Possibly related PRs
Suggested reviewers
Poem
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
application/__init__.py(1 hunks)
🔇 Additional comments (3)
application/__init__.py (3)
35-35: Good change to fix the logger initialization timing.Moving the logger initialization after the configuration is applied ensures the logger uses the proper configuration. This addresses the issue where the logger was being cached before configurations were applied.
39-40: Well-structured exception handling.Initializing the logger in the exception branch is consistent with the success path and ensures that exception messages are properly logged with the fallback configuration.
35-42:❓ Verification inconclusive
Verify logger behavior with unit tests.
The changes look good, but it would be beneficial to add unit tests that verify the logger's behavior with different configuration scenarios to ensure the fixes work as expected.
🌐 Web query:
How to test Python logging configuration in unit tests?💡 Result:
To effectively test Python logging configurations in unit tests, developers can leverage built-in testing utilities from both the standard library and third-party frameworks. Here are the primary approaches:
Using unittest's assertLogs
The
unittest.TestCaseclass provides anassertLogscontext manager to capture log messages during tests:import unittest import logging class LoggingTest(unittest.TestCase): def test_log_messages(self): logger = logging.getLogger('test_logger') with self.assertLogs(logger, level='DEBUG') as cm: logger.debug('Debug message') logger.error('Error message') self.assertEqual(cm.output, [ 'DEBUG:test_logger:Debug message', 'ERROR:test_logger:Error message' ])Key features:
- Captures all log messages at or above specified level
- Returns a LogCapture object containing
output(formatted messages) andrecords(raw log records)- Supports testing logger names and log levels[1][4][8]
Using pytest's caplog Fixture
For pytest users, the
caplogfixture provides comprehensive logging test capabilities:def test_log_messages(caplog): caplog.set_level(logging.INFO) logger = logging.getLogger('test_logger') logger.info('Information message') logger.warning('Warning message') assert len(caplog.records) == 2 assert caplog.records[0].message == 'Information message' assert 'Warning' in caplog.textKey features:
- Access raw records through
caplog.records- Get formatted text via
caplog.text- Set different log levels using
caplog.set_level()[4][6]Best Practices
- Test Message Formatting: Verify formatted messages include variables correctly
logger.info('User %s logged in', 'john') assert 'User john logged in' in caplog.text[4]
- Check Log Levels: Confirm appropriate severity levels are used
assert caplog.records[0].levelname == 'ERROR'[4]
Test Multiple Handlers: Use
logging.NullHandleror mock handlers to verify handler configurations[2]Avoid Over-Mocking: Prefer capturing actual log output instead of mocking logger methods to test full logging pipeline[4]
Approach Framework Advantages Limitations assertLogs unittest Native integration, context manager Requires class inheritance caplog fixture pytest Flexible level control, rich API Requires pytest Custom log handler Any Full control over log destinations More setup required[2] For complex logging scenarios (e.g. structured logging), consider using dedicated testing libraries like Eliot[5]. When testing logging configuration itself, verify handler registration and formatters by inspecting the logger's handlers and their configurations[7].
Citations:
- 1: https://monicagranbois.com/blog/python/til-how-to-unit-test-a-log-statement-in-python/
- 2: https://github.com/airtower-luna/python-log-test-example
- 3: https://www.youtube.com/watch?v=bMk5a10KeVc
- 4: https://browniebroke.com/blog/2024-12-13-testing-your-python-logging-like-a-boss/
- 5: https://eliot.readthedocs.io/en/stable/generating/testing.html
- 6: https://docs.pytest.org/en/stable/how-to/logging.html
- 7: https://docs.python.org/3/howto/logging.html
- 8: https://docs.python.org/3/library/unittest.html
Unit Test Logger Behavior
The changes in the logger setup look solid. However, to ensure robust behavior in all configuration scenarios, please add unit tests that verify the logger's output. For example:
- Exception path: When an exception is raised during logger setup, validate that the fallback configuration is applied and that an error is logged via
logger.exception("Logging setup failed").- Success path: When no exception occurs, confirm that the logger issues a warning with the expected message format (e.g., "Logging setup is completed with config=%s").
You can leverage testing utilities such as
unittest.TestCase’sassertLogscontext manager or pytest’scaplogfixture to capture and assert log messages. This will provide confidence that the logger behaves as intended under both successful and failure scenarios.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
application/__init__.py(1 hunks)
🔇 Additional comments (2)
application/__init__.py (2)
33-33: LGTM - Proper logger initialization after configuration.The logger is now correctly initialized after the logging configuration has been applied via
dictConfig. This ensures the logger adopts the intended configuration rather than using cached settings.
37-37: LGTM - Consistent logger initialization in exception handler.Good implementation of logger initialization in the exception handling block, ensuring the logger is properly configured even when loading the YAML configuration fails.
| except Exception: | ||
| # Fallback to a basic configuration | ||
| logging.basicConfig(format='%(asctime)s %(levelname)s [%(name)s:%(lineno)d] %(message)s', level=logging.INFO, force=True) | ||
| logger = logging.getLogger(__name__) | ||
| logger.exception("Logging setup failed") | ||
| else: | ||
| logger.warning("Logging setup is completed with config=%s", config) |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Consider limiting logged configuration details.
Logging the entire configuration object could potentially expose sensitive information (like log file paths, credentials in custom handlers) and create verbose log entries.
- logger.warning("Logging setup is completed with config=%s", config)
+ logger.warning("Logging setup completed successfully")📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| logger.warning("Logging setup is completed with config=%s", config) | |
| logger.warning("Logging setup completed successfully") |
Description
Logic changes to logging initialization and configuration in
application/__init__.py. This was done to fix PR#42 (merged). Apparently, the logger initialization in PR#42 leads to ambiguous behaviours because the logger is retrieved from the cache upon executing a logging statement. Since the logger is declared and initialized before any logging configurations, i.e dictConfig in the successful case and the basicConfig in the failed case, logging statements actually end up not picking up these configurations because of the previously cachced logger. This PR makes minor modifications to ensure that the logger picks up the correct configurations after loading the configurations.Summary by CodeRabbit
Summary by CodeRabbit