Skip to content

refactor: Unify .env loading logic, eliminate code duplication#338

Open
SHINING2024 wants to merge 1 commit into
calesthio:mainfrom
SHINING2024:refactor/unify-env-loader
Open

refactor: Unify .env loading logic, eliminate code duplication#338
SHINING2024 wants to merge 1 commit into
calesthio:mainfrom
SHINING2024:refactor/unify-env-loader

Conversation

@SHINING2024

Copy link
Copy Markdown

Overview

Unify the .env file loading logic across the OpenMontage project, eliminating code duplication between base_tool.py and tool_registry.py while maintaining full backward compatibility.

Motivation

  • Code Duplication: Identical .env loading logic (~40 lines) exists in both tools/base_tool.py and tools/tool_registry.py
  • Maintenance Burden: Changes require updates in two places, easy to miss one
  • Underutilized Module: lib/env_loader.py already exists but wasn't being fully leveraged

Technical Implementation

1. Enhanced lib/env_loader.py

  • Added _load_env_manual() function with exact same manual parsing logic as original code
  • Kept optional python-dotenv support via use_dotenv parameter
  • Uses manual parsing by default to ensure identical behavior to original code
  • All existing APIs (load_env(), get_env(), require_env()) remain unchanged

2. Refactored tools/base_tool.py

  • Removed _load_dotenv() function (lines 25-60)
  • Changed to import from lib.env_loader import load_env
  • Calls load_env() instead of the removed _load_dotenv()

3. Refactored tools/tool_registry.py

  • Removed _load_dotenv() static method (lines 87-116)
  • Added import from lib.env_loader import load_env
  • Calls load_env() instead of self._load_dotenv() in discover() method

Compatibility Guarantees

Functionally Identical: Manual parsing logic matches original line-by-line
No Breaking Changes: All public APIs remain unchanged
No New Dependencies: Doesn't introduce any additional Python packages
Backward Compatible: Existing .env file formats fully supported

Testing Recommendations

  1. Run existing project test suite
  2. Verify tool registry discovery works correctly
  3. Test that tools with API keys can read environment variables properly
  4. Verify consistent behavior across Windows/macOS/Linux platforms

Files Modified

  1. lib/env_loader.py - Enhanced functionality
  2. tools/base_tool.py - Removed duplicate code
  3. tools/tool_registry.py - Removed duplicate code

- Enhance lib/env_loader.py with manual parsing logic for full compatibility
- Remove duplicate _load_dotenv() function from tools/base_tool.py
- Remove duplicate _load_dotenv() static method from tools/tool_registry.py
- Replace duplicate implementations with unified load_env()
- Maintain all existing functionality and behavior unchanged
- No new dependencies introduced, no breaking changes#
@SHINING2024 SHINING2024 requested a review from calesthio as a code owner July 9, 2026 13:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant