-
Notifications
You must be signed in to change notification settings - Fork 669
Provide step information in additional log messages #4484
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
Conversation
…er and command classes
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.
Pull Request Overview
This PR addresses a caching issue with loggers in Molecule's core components that prevented them from displaying dynamic scenario and step context during execution. The solution converts static logger instance variables to dynamic properties that retrieve fresh context on each access.
- Convert logger instance variables to @Property methods across multiple classes
- Each property now dynamically retrieves current scenario and step context using getattr
- Enable consistent log message formatting with scenario and step information
Reviewed Changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 6 comments.
Show a summary per file
File | Description |
---|---|
src/molecule/dependency/base.py | Converts _log from instance variable to property for dynamic context retrieval |
src/molecule/dependency/shell.py | Removes static logger initialization, inherits dynamic logger from base |
src/molecule/dependency/ansible_galaxy/base.py | Removes static logger initialization, inherits dynamic logger from base |
src/molecule/provisioner/ansible.py | Converts _log to property and removes init method |
src/molecule/provisioner/ansible_playbook.py | Converts _log from instance variable to property |
src/molecule/provisioner/ansible_playbooks.py | Converts _log from instance variable to property |
src/molecule/verifier/ansible.py | Converts _log to property and removes init method |
src/molecule/verifier/testinfra.py | Converts _log from instance variable to property |
src/molecule/command/create.py | Converts _log to property and removes init method |
src/molecule/command/destroy.py | Converts _log to property and removes init method |
tests/unit/dependency/test_shell.py | Updates test to work with dynamic logger context |
Command classes will be handled separately with base.Base consolidation. This PR now focuses only on dependency, provisioner, and verifier classes.
…-aware logging (#4488) ## Summary Comprehensive enhancement of Config class and command base logging with scenario-aware context, building on the foundation from merged PRs #4483 and #4484. ## User-Facing Changes **Enhanced logging context across all Config and command operations:** ### Before: Generic logging without context ``` DEBUG Validating schema molecule.yml WARNING Driver 'default' is currently in use but config defines 'podman' INFO Performing prerun with role_name_check=0... ``` ### After: Complete scenario and step context ``` DEBUG smoke ➜ validate: Validating schema /path/to/molecule.yml WARNING smoke ➜ config: Driver 'default' is currently in use but config defines 'podman' INFO smoke ➜ prerun: Performing prerun with role_name_check=0... INFO smoke ➜ discovery: scenario test matrix: syntax ``` ## Technical Implementation **🏗️ Config Class Enhancements:** - Add reusable `@property _log` method to Config class for dynamic scenario context - Convert all Config logging calls to use scenario-aware loggers - Enhanced validation logging with hardcoded scenario context during bootstrap - Improved galaxy file and driver validation warnings with full context **🧹 Command Base Logger Consolidation:** - Replace 5 scattered `get_scenario_logger()` calls with single global `_log()` function - Unified logging pattern: `_log(scenario_name, step, message, level='info')` - Pre-formatted messages eliminate complex placeholder handling - Cleaner, more maintainable logger creation across all command operations **✅ Enhanced Test Coverage:** - Updated `test_validate` to use `caplog` instead of mock patching - Validates scenario logger extras: `molecule_scenario` and `molecule_step` - Robust verification of message content and context attributes ## Changes Included **Config Class (4 commits):** 1. `feat: add step context to command base logger calls` 2. `feat: add scenario context to config file modification warning` 3. `refactor: add reusable _log property to Config class` 4. `refactor: convert all Config logging to scenario-aware logging` **Command Base Improvements (2 commits):** 5. `refactor: replace scattered logger initializations with global _log function` 6. `fix: update test_validate to use caplog and check scenario logger extras` **Dependencies (Included):** - Builds on: All cleanup commits from PR #4487 (unused logger removal) ## Dependencies - **Depends on**: PR #4487 (unused logger cleanup) - **Builds on**: PR #4484 (merged - dynamic properties for dependency/provisioner/verifier) - **Foundation**: PR #4483 (merged - enhanced logging adapter with dual-output) ## Testing - ✅ **Comprehensive**: All config tests passing (45/45) - ✅ **Enhanced coverage**: Validates scenario logger extras in test suite - ✅ **Functional verification**: `molecule syntax` confirms enhanced logging works - ✅ **Backwards compatible**: No breaking changes to existing functionality ## Impact - **Complete Config context**: All Config operations now include scenario and step information - **Unified command logging**: Consistent pattern across all command base operations - **Better maintainability**: Single global `_log` function replaces scattered initializations - **Enhanced debugging**: Full context for configuration validation and driver operations --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Problem: Several core components (dependency, provisioner, verifier, and commands) cache their loggers at initialization, preventing them from picking up dynamic scenario and step context during execution.
Solution: Converted self._log from instance variable to @Property in multiple classes:
Each property now dynamically retrieves fresh scenario and step context on access using getattr(self._config, 'action', default_step).
Impact: Log messages from these components now consistently show the current scenario and step context in the format 'scenario ➜ step: message'.
Testing: Verified via integration tests across multiple scenarios and commands. The changes maintain compatibility with the dual-output logging system introduced in PR #4483.