Skip to content

Conversation

@cidrblock
Copy link
Contributor

@cidrblock cidrblock commented Jul 28, 2025

Summary

Remove unused LOG = logging.getLogger(__name__) definitions across the Molecule codebase.

Changes

🧹 Pure Cleanup (Zero Functionality Changes):

  • Remove 20+ unused LOG = logging.getLogger(__name__) definitions
  • Remove unnecessary import logging statements where applicable

…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
Copy link
Contributor

Copilot AI left a 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 logging statements 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 cidrblock merged commit edac329 into ansible:main Jul 28, 2025
19 checks passed
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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

1 participant