cleanup: remove unused logger definitions across codebase#4487
Merged
cidrblock merged 4 commits intoansible:mainfrom Jul 28, 2025
Merged
cleanup: remove unused logger definitions across codebase#4487cidrblock merged 4 commits intoansible:mainfrom
cidrblock merged 4 commits intoansible:mainfrom
Conversation
…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
Contributor
There was a problem hiding this comment.
Pull Request Overview
This PR removes unused logger definitions (LOG = logging.getLogger(__name__)) and associated import statements across the Molecule codebase. This is a pure cleanup effort that eliminates dead code without changing any functionality.
Key changes:
- Remove 20+ unused
LOG = logging.getLogger(__name__)definitions - Remove unnecessary
import loggingstatements where no logging is used - Clean up import organization in affected files
Reviewed Changes
Copilot reviewed 24 out of 24 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| src/molecule/verifier/testinfra.py | Remove unused logger and import |
| src/molecule/state.py | Remove unused logger and import |
| src/molecule/provisioner/ansible_playbooks.py | Remove unused logger and import |
| src/molecule/platforms.py | Remove unused logger and import |
| src/molecule/logger.py | Remove unused logger definition |
| src/molecule/driver/delegated.py | Remove unused logger and import |
| src/molecule/dependency/shell.py | Remove unused logger and import |
| src/molecule/dependency/ansible_galaxy/roles.py | Remove unused logger and import |
| src/molecule/dependency/ansible_galaxy/collections.py | Remove unused logger and import |
| src/molecule/dependency/ansible_galaxy/base.py | Remove unused logger and import |
| src/molecule/command/verify.py | Remove unused logger and import |
| src/molecule/command/test.py | Remove unused logger and import |
| src/molecule/command/syntax.py | Remove unused logger and import |
| src/molecule/command/reset.py | Remove unused logger and import |
| src/molecule/command/matrix.py | Remove unused logger and import |
| src/molecule/command/list.py | Remove unused logger and import |
| src/molecule/command/init/scenario.py | Remove unused logger and import |
| src/molecule/command/init/init.py | Remove unused logger and import |
| src/molecule/command/init/base.py | Remove unused logger and import |
| src/molecule/command/drivers.py | Remove unused logger and import |
| src/molecule/command/dependency.py | Remove unused logger and import |
| src/molecule/command/converge.py | Remove unused logger and import |
| src/molecule/command/check.py | Remove unused logger and import |
| src/molecule/command/base.py | Remove unused logger definition |
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>
cidrblock
added a commit
that referenced
this pull request
Jul 28, 2025
Depends on PR #4487 Problem: Command classes had duplicate logger setup code across 7+ files. Solution: Single @Property _log in base.Base with automatic step derivation. Key Changes: - Added @Property _log to base.Base with automatic step naming - Removed individual logger setups from 7 command classes - Eliminated 50+ lines of duplicate code - Updated tests for scenario logger pattern Impact: - Future command classes get logger automatically - Consistent scenario->step format across all commands - Perfect architectural separation of concerns Testing: All command tests pass with proper context validation. --------- 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.
Summary
Remove unused
LOG = logging.getLogger(__name__)definitions across the Molecule codebase.Changes
🧹 Pure Cleanup (Zero Functionality Changes):
LOG = logging.getLogger(__name__)definitionsimport loggingstatements where applicable