-
Notifications
You must be signed in to change notification settings - Fork 665
cleanup: remove unused logger definitions across codebase #4487
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
cleanup: remove unused logger definitions across codebase #4487
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
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 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 |
…-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>
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>
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