Provide step information in additional log messages#4484
Merged
cidrblock merged 3 commits intoansible:mainfrom Jul 28, 2025
Merged
Provide step information in additional log messages#4484cidrblock merged 3 commits intoansible:mainfrom
cidrblock merged 3 commits intoansible:mainfrom
Conversation
…er and command classes
Contributor
There was a problem hiding this comment.
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.
alisonlhart
approved these changes
Jul 28, 2025
This was referenced Jul 28, 2025
cidrblock
added a commit
that referenced
this pull request
Jul 28, 2025
…-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>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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.