fix: centralize logging config in main.py (#26) #46

cal · 2026-03-04T00:06:49Z

cal commented

2026-03-04 00:06:49 +00:00

Fixes #26

Summary

Added single logging.basicConfig() call to app/main.py — the canonical entry point
Removed duplicate (no-op) basicConfig calls from app/db_engine.py, app/dependencies.py, and all 30 router files in app/routers_v2/
Removed now-unused LOG_DATA dict and associated date/log_level locals from dependencies.py and db_engine.py

What was wrong

logging.basicConfig() is a no-op after the first call. db_engine.py was always imported first (by main.py), so its call won configuration. All 30+ router calls were silently discarded, and effective logging behavior was determined by import order rather than explicit intent.

Files changed

app/main.py — added centralized logging setup
app/db_engine.py — removed basicConfig, date, log_level locals
app/dependencies.py — removed LOG_DATA dict and basicConfig call
app/routers_v2/*.py (30 files) — removed basicConfig calls and LOG_DATA from imports

Fixes #26 ## Summary - Added single `logging.basicConfig()` call to `app/main.py` — the canonical entry point - Removed duplicate (no-op) `basicConfig` calls from `app/db_engine.py`, `app/dependencies.py`, and all 30 router files in `app/routers_v2/` - Removed now-unused `LOG_DATA` dict and associated `date`/`log_level` locals from `dependencies.py` and `db_engine.py` ## What was wrong `logging.basicConfig()` is a no-op after the first call. `db_engine.py` was always imported first (by `main.py`), so its call won configuration. All 30+ router calls were silently discarded, and effective logging behavior was determined by import order rather than explicit intent. ## Files changed - `app/main.py` — added centralized logging setup - `app/db_engine.py` — removed basicConfig, date, log_level locals - `app/dependencies.py` — removed LOG_DATA dict and basicConfig call - `app/routers_v2/*.py` (30 files) — removed basicConfig calls and LOG_DATA from imports

cal added 1 commit 2026-03-04 00:06:50 +00:00

fix: centralize logging config in main.py — remove basicConfig from 32 files (#26 ) 867e8ce888

Moved logging.basicConfig() to app/main.py as the single source of truth.
Removed duplicate (no-op) calls from app/db_engine.py, app/dependencies.py,
and all 30 router files in app/routers_v2/. Removed the now-unused LOG_DATA
dict and date/log_level locals from dependencies.py and db_engine.py.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

cal added the

ai-reviewing

label 2026-03-04 00:16:29 +00:00

cal reviewed 2026-03-04 00:18:52 +00:00

cal left a comment

AI Code Review

Files Reviewed

app/main.py (modified — centralized logging setup added)
app/db_engine.py (modified — removed duplicate basicConfig)
app/dependencies.py (modified — removed LOG_DATA and duplicate basicConfig)
app/routers_v2/*.py (30 files — removed LOG_DATA imports and duplicate basicConfig calls)

Findings

Correctness

The fix is technically sound. logging.basicConfig() is indeed a no-op after the first call, and placing the single authoritative call in main.py — before from .db_engine import db and the router imports — ensures it executes first and actually configures the root logger.
The old code used a bare string "WARN" as the log level in db_engine.py and dependencies.py. The new main.py correctly uses logging.WARNING (the integer constant). This is a correctness improvement.
All 30 router files retain import logging — verified that each still makes direct logging.* calls in their route handlers, so no orphaned imports were introduced.
dependencies.py retains import logging, import os, and import datetime — all three remain in active use by db_get, db_patch, db_post, db_delete, and int_timestamp after LOG_DATA removal.
db_engine.py retains import logging and import os — both are still used by the remainder of the file.

Security

No issues found. No secrets, hardcoded credentials, or injection vectors introduced.

Style & Conventions

The _log_date underscore prefix is a good convention for a module-level private temporary.
The db.create_tables() reformatting to multi-line list is a cosmetic improvement that follows standard Python style.
Minor (non-blocking): The date format f"{datetime.now().year}-{datetime.now().month}-{datetime.now().day}" produces single-digit months/days (e.g. 2026-3-3). This is unchanged from the pre-existing format in db_engine.py, so not a regression introduced here.

Suggestions

No blocking suggestions. If log filename padding is ever desired, datetime.now().strftime("%Y-%m-%d") would give zero-padded dates, but that's out of scope for this PR.

Verdict: APPROVED ✓

Clean, well-scoped fix. The root cause (import-order-dependent logging config) is correctly addressed by moving the single basicConfig call to main.py before any sub-module imports execute. No correctness, security, or convention issues found. Ready to merge.

Automated review by Claude PR Reviewer

## AI Code Review ### Files Reviewed - `app/main.py` (modified — centralized logging setup added) - `app/db_engine.py` (modified — removed duplicate basicConfig) - `app/dependencies.py` (modified — removed LOG_DATA and duplicate basicConfig) - `app/routers_v2/*.py` (30 files — removed LOG_DATA imports and duplicate basicConfig calls) ### Findings #### Correctness - The fix is technically sound. `logging.basicConfig()` is indeed a no-op after the first call, and placing the single authoritative call in `main.py` — before `from .db_engine import db` and the router imports — ensures it executes first and actually configures the root logger. - The old code used a bare string `"WARN"` as the log level in `db_engine.py` and `dependencies.py`. The new `main.py` correctly uses `logging.WARNING` (the integer constant). This is a correctness improvement. - All 30 router files retain `import logging` — verified that each still makes direct `logging.*` calls in their route handlers, so no orphaned imports were introduced. - `dependencies.py` retains `import logging`, `import os`, and `import datetime` — all three remain in active use by `db_get`, `db_patch`, `db_post`, `db_delete`, and `int_timestamp` after `LOG_DATA` removal. - `db_engine.py` retains `import logging` and `import os` — both are still used by the remainder of the file. #### Security - No issues found. No secrets, hardcoded credentials, or injection vectors introduced. #### Style & Conventions - The `_log_date` underscore prefix is a good convention for a module-level private temporary. - The `db.create_tables()` reformatting to multi-line list is a cosmetic improvement that follows standard Python style. - Minor (non-blocking): The date format `f"{datetime.now().year}-{datetime.now().month}-{datetime.now().day}"` produces single-digit months/days (e.g. `2026-3-3`). This is unchanged from the pre-existing format in `db_engine.py`, so not a regression introduced here. #### Suggestions - No blocking suggestions. If log filename padding is ever desired, `datetime.now().strftime("%Y-%m-%d")` would give zero-padded dates, but that's out of scope for this PR. ### Verdict: APPROVED ✓ Clean, well-scoped fix. The root cause (import-order-dependent logging config) is correctly addressed by moving the single `basicConfig` call to `main.py` before any sub-module imports execute. No correctness, security, or convention issues found. **Ready to merge.** --- *Automated review by Claude PR Reviewer*

cal added the

ai-reviewed

label 2026-03-04 00:19:28 +00:00

cal force-pushed ai/paper-dynasty-database#26 from 867e8ce888 to 053fcbab05

2026-03-05 03:22:06 +00:00

Compare

cal merged commit 341296b573 into next-release

2026-03-05 03:22:29 +00:00

cal deleted branch ai/paper-dynasty-database#26

2026-03-05 03:22:30 +00:00

cal referenced this issue from a commit

2026-03-05 03:22:30 +00:00

Merge pull request 'fix: centralize logging config in main.py (#26)' (#46) from ai/paper-dynasty-database#26 into next-release

Sign in to join this conversation.

No reviewers

No Milestone

No project

No Assignees

1 Participants

Notifications

Due Date

The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: cal/paper-dynasty-database#46