refactor: enhance Config and command base with comprehensive scenario-aware logging by cidrblock · Pull Request #4488 · ansible/molecule

cidrblock · 2025-07-28T20:16:04Z

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):

feat: add step context to command base logger calls
feat: add scenario context to config file modification warning
refactor: add reusable _log property to Config class
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 cleanup: remove unused logger definitions across codebase #4487 (unused logger removal)

Dependencies

Depends on: PR cleanup: remove unused logger definitions across codebase #4487 (unused logger cleanup)
Builds on: PR Provide step information in additional log messages #4484 (merged - dynamic properties for dependency/provisioner/verifier)
Foundation: PR feat: enhance logging with consistent scenario and step information #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

…and state.py - Remove unused LOG = logging.getLogger(__name__) definitions - Remove corresponding logging imports where no longer needed - Fix Callable import in state.py to use collections.abc

- Remove unused LOG = logging.getLogger(__name__) from command modules - Remove unused LOG definitions from dependency, driver, and provisioner modules - Remove corresponding logging imports where no longer needed - Clean up state.py, platforms.py, verifier/testinfra.py and others - All pre-commit checks passing after cleanup

- Remove unused LOG = logging.getLogger(__name__) from command/base.py - Keep logging import as it's still used in one place (line 137) - All LOG definitions are now used (verified 6 remaining files)

- Remove environment detection test cases (not part of unused logger cleanup) - Remove TTY detection test function (belongs in separate environment detection PR) - Keep focus on pure unused logger cleanup only

- Add step context to scenario discovery logging (discovery) - Add step context to prerun logging (prerun) - Add step context to reset scenario logging (reset) - Add step context to error cleanup logging (action/cleanup) - Add step context to default scenario fallback warning (subcommand)

- Replace LOG.warning with scenario logger in Config.state property - Add step context 'config' for configuration-related warnings - Import logger module for scenario context support

Add @Property _log method to Config class that provides scenario-aware logging with 'config' step context. Update existing logger call in state property to use self._log instead of creating ad-hoc logger. This enables other Config methods to use self._log for consistent scenario and step context in log messages.

Complete conversion of Config class logging to use scenario context: - _validate(): Use hardcoded scenario logger during validation - _get_driver_name(): Use self._log for driver warnings - collection(): Use self._log for galaxy.yml validation warnings Remove unused module-level logger (LOG) and import since all logging now uses scenario-aware loggers with proper step context. Benefits: - All Config logging now includes scenario and step information - Consistent logging pattern throughout Config class - Eliminates context-less logging in Config operations

…unction - Add global _log() function in base.py for on-demand logger creation - Replace 5 manual get_scenario_logger() calls with single reusable function - Simplify logger usage: _log(scenario_name, step, message, level='info') - Pre-format messages instead of using format placeholders - Cleaner, more maintainable logging pattern across command base

- Replace patched_logger_debug mock with caplog for better testing approach - Verify debug message content includes molecule file path - Check scenario logger extras: molecule_scenario and molecule_step - Ensure validate step is correctly recorded in log record - More robust testing of scenario-aware logging functionality

Copilot

Pull Request Overview

This PR enhances the Config class and command base with comprehensive scenario-aware logging by consolidating scattered logger calls into a unified pattern with full scenario and step context throughout all operations.

Adds scenario-aware logging context to Config operations including validation, driver checks, and file modifications
Replaces scattered get_scenario_logger() calls with a single global _log() function in command base
Updates test suite to use caplog and verify scenario logger extras

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File	Description
src/molecule/config.py	Adds `_log` property for scenario-aware logging and converts all Config logging calls to use contextual loggers
src/molecule/command/base.py	Introduces global `_log()` function to replace scattered logger initializations and standardize logging pattern
tests/unit/test_config.py	Updates test to use `caplog` instead of mocking and validates scenario logger extras

src/molecule/command/base.py

…-properties - Merge PR ansible#4488 comprehensive work into PR ansible#4486 - Includes Config class scenario-aware logging - Includes global _log function for command base - Includes all test improvements and documentation

cidrblock added 3 commits July 28, 2025 13:12

cleanup: remove unused logger definitions from logger.py, syntax.py, …

6ddf981

…and state.py - Remove unused LOG = logging.getLogger(__name__) definitions - Remove corresponding logging imports where no longer needed - Fix Callable import in state.py to use collections.abc

cleanup: remove final unused LOG definition from base.py

0e5fcd9

- Remove unused LOG = logging.getLogger(__name__) from command/base.py - Keep logging import as it's still used in one place (line 137) - All LOG definitions are now used (verified 6 remaining files)

cidrblock requested a review from a team as a code owner July 28, 2025 20:16

github-project-automation bot added this to 🧰 devtools project board Jul 28, 2025

cidrblock had a problem deploying to ack July 28, 2025 20:16 — with GitHub Actions Failure

cidrblock added 5 commits July 28, 2025 13:22

cleanup: remove out-of-scope test additions from test_ansi_output.py

b609628

- Remove environment detection test cases (not part of unused logger cleanup) - Remove TTY detection test function (belongs in separate environment detection PR) - Keep focus on pure unused logger cleanup only

feat: add scenario context to config file modification warning

f1c4d95

- Replace LOG.warning with scenario logger in Config.state property - Add step context 'config' for configuration-related warnings - Import logger module for scenario context support

cidrblock force-pushed the refactor/dynamic-logger-properties branch from 654cf7b to 44c8d6e Compare July 28, 2025 20:26

cidrblock had a problem deploying to ack July 28, 2025 20:26 — with GitHub Actions Failure

cidrblock had a problem deploying to ack July 28, 2025 20:41 — with GitHub Actions Failure

cidrblock force-pushed the refactor/dynamic-logger-properties branch from d2a65b1 to bd2fb4e Compare July 28, 2025 20:44

cidrblock had a problem deploying to ack July 28, 2025 20:44 — with GitHub Actions Failure

Merge branch 'main' into refactor/dynamic-logger-properties

c45b9d0

cidrblock had a problem deploying to ack July 28, 2025 20:45 — with GitHub Actions Failure

chore: auto fixes from pre-commit.com hooks

8e18556

cidrblock added the skip-changelog label Jul 28, 2025

pre-commit-ci bot had a problem deploying to ack July 28, 2025 20:46 Error

cidrblock temporarily deployed to ack July 28, 2025 20:51 — with GitHub Actions Inactive

cidrblock temporarily deployed to ack July 28, 2025 20:53 — with GitHub Actions Inactive

cidrblock changed the title ~~refactor: enhance Config class with scenario-aware logging~~ refactor: enhance Config and command base with comprehensive scenario-aware logging Jul 28, 2025

cidrblock requested a review from Copilot July 28, 2025 21:00

Copilot AI reviewed Jul 28, 2025

View reviewed changes

src/molecule/command/base.py Show resolved Hide resolved

rm comment

3be5d22

cidrblock temporarily deployed to ack July 28, 2025 21:08 — with GitHub Actions Inactive

cidrblock enabled auto-merge (squash) July 28, 2025 21:19

cidrblock disabled auto-merge July 28, 2025 21:19

cidrblock merged commit 1ef06d8 into ansible:main Jul 28, 2025
21 checks passed

github-project-automation bot moved this to Done in 🧰 devtools project board Jul 28, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

refactor: enhance Config and command base with comprehensive scenario-aware logging#4488

refactor: enhance Config and command base with comprehensive scenario-aware logging#4488
cidrblock merged 13 commits intoansible:mainfrom
cidrblock:refactor/dynamic-logger-properties

cidrblock commented Jul 28, 2025 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

cidrblock commented Jul 28, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

User-Facing Changes

Before: Generic logging without context

After: Complete scenario and step context

Technical Implementation

Changes Included

Dependencies

Testing

Impact

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

cidrblock commented Jul 28, 2025 •

edited

Loading