refactor(P4a): stop reconfiguring root logging at import in 20 modules#271
Merged
Merged
Conversation
Structural refactor phase P4 (backend#796), first slice. Twenty modules (every validator + database + file_transfer) called setup_logging(config) at IMPORT time — reconfiguring the root logger's handlers globally, 20x on every import: an import-time side effect a library shouldn't have. The entrypoints (cli/run.main and the template scripts) already call setup_logging() at startup, so the per-module calls were redundant. Removes the import-time setup_logging(config) call + its now-unused import from all 20 modules. Each module's `config = Config()` and `logger.setLevel(config.LOG_LEVEL)` are kept (full config-by-construction + the env-var-bridge removal are the next P4 slices). Logging is now configured once, at the app entrypoint. Behaviour-preserving: root logging is still configured by the entrypoint; child loggers inherit. Full unit suite 1078 passed, 96.9% coverage; e2e (real MySQL) 23 passed, 1 xpassed — characterization goldens unchanged. Next: P4b (inject Config into the file validators) and P4c (file_transfer takes paths; delete the cli/run env-var bridge) — the config-by-construction core, now gateable with the local e2e harness. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Collaborator
Author
|
👋 Heads-up — Code review queue is at 11 / 8 Above the WIP limit. The team convention is to review existing PRs before opening new work. Open PRs currently in Code review (oldest first):
Pull from review before opening new work. (This is a nudge from the kanban WIP check, not a block.) |
This was referenced Jun 16, 2026
divyasinghds
approved these changes
Jun 16, 2026
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
Structural refactor — phase P4 (config injection), first slice. Twenty modules (every validator +
database+file_transfer) calledsetup_logging(config)at import time — reconfiguring the root logger's handlers globally, 20× on every import. That's an import-time side effect a library shouldn't have. The entrypoints (cli/run.mainand the template scripts) already callsetup_logging()at startup, so the per-module calls were redundant.This removes the import-time
setup_logging(config)call + its now-unused import from all 20 modules. Each module keepsconfig = Config()andlogger.setLevel(config.LOG_LEVEL)— the full config-by-construction + env-var-bridge removal are the next P4 slices (below). Logging is now configured once, at the app entrypoint.Behaviour preservation
Root logging is still configured by the entrypoint; child loggers inherit. Diff is exactly the 20 files (just the 2-line removal each).
What's next in P4
This was the safe, e2e-independent slice. The config-by-construction core follows:
Configinto the file validators (via the registry'sbuild_validators) instead of the module-level global.file_transfertakes resolved paths as parameters; deletecli/run._set_legacy_env_vars(theos.environbridge). The e2e-sensitive part — now gateable with the local harness (Docker confirmed working).(Pre-existing flake8 noise in some touched files — unused
List/Optional/pdetc. — is unrelated and left out of scope.)🤖 Generated with Claude Code