566a07d3a4
- Extract compose.py from traefik.py for generic compose parsing (env loading, interpolation, ports, volumes, networks) - Rename ssh.py to executor.py for clarity - Extract operations.py from cli.py for business logic (up_services, discover_running_services, preflight checks) - Update CLAUDE.md with new architecture diagram - Add docs/dev/future-improvements.md for low-priority items CLI is now a thin layer that delegates to operations module. All 70 tests pass.
3.8 KiB
Future Improvements
Low-priority improvements identified during code review. These are not currently causing issues but could be addressed if they become pain points.
1. State Module Efficiency (LOW)
Current: Every state operation reads and writes the entire file.
def set_service_host(config, service, host):
state = load_state(config) # Read file
state[service] = host
save_state(config, state) # Write file
Impact: With 87 services, this is fine. With 1000+, it would be slow.
Potential fix: Add batch operations:
def update_state(config, updates: dict[str, str | None]) -> None:
"""Batch update: set services to hosts, None means remove."""
state = load_state(config)
for service, host in updates.items():
if host is None:
state.pop(service, None)
else:
state[service] = host
save_state(config, state)
When to do: Only if state operations become noticeably slow.
2. Remote-Aware Compose Path Resolution (LOW)
Current: config.get_compose_path() checks if files exist on the local filesystem:
def get_compose_path(self, service: str) -> Path:
for filename in ("compose.yaml", "compose.yml", ...):
candidate = service_dir / filename
if candidate.exists(): # Local check!
return candidate
Why this works: NFS/shared storage means local = remote.
Why it could break: If running compose-farm from a machine without the NFS mount, it returns compose.yaml (the default) even if docker-compose.yml exists on the remote host.
Potential fix: Query the remote host for file existence, or accept this limitation and document it.
When to do: Only if users need to run compose-farm from non-NFS machines.
3. Add Integration Tests for CLI Commands (MEDIUM)
Current: No integration tests for the actual CLI commands. Tests cover the underlying functions but not the Typer commands themselves.
Potential fix: Add integration tests using CliRunner from Typer:
from typer.testing import CliRunner
from compose_farm.cli import app
runner = CliRunner()
def test_check_command_validates_config():
result = runner.invoke(app, ["check", "--local"])
assert result.exit_code == 0
When to do: When CLI behavior becomes complex enough to warrant dedicated testing.
4. Add Tests for operations.py (MEDIUM)
Current: Operations module has 30% coverage. Most logic is tested indirectly through test_sync.py.
Potential fix: Add dedicated tests for:
up_services()with migration scenariospreflight_check()check_host_compatibility()
When to do: When adding new operations or modifying migration logic.
5. Consider Structured Logging (LOW)
Current: Operations print directly to console using Rich. This couples the operations module to the Rich library.
Potential fix: Use Python's logging module with a custom Rich handler:
import logging
logger = logging.getLogger(__name__)
# In operations:
logger.info("Migrating %s from %s to %s", service, old_host, new_host)
# In cli.py - configure Rich handler:
from rich.logging import RichHandler
logging.basicConfig(handlers=[RichHandler()])
Benefits:
- Operations become testable without capturing stdout
- Logs can be redirected to files
- Log levels provide filtering
When to do: Only if console output coupling becomes a problem for testing or extensibility.
Design Decisions to Keep
These patterns are working well and should be preserved:
- asyncio + asyncssh - Solid async foundation
- Pydantic models - Clean validation
- Rich for output - Good UX
- Test structure - Good coverage
- Module separation - cli/operations/executor/compose pattern
- KISS principle - Don't over-engineer