cal/major-domo-database
1
0
Fork 0
Code Issues 25 Pull Requests 25 Actions Packages Projects Releases Wiki Activity

fix: remove SQLite fallback code from db_engine.py (#70) #89

Open
Claude wants to merge 1 commits from issue/70-remove-sqlite-fallback-code-from-db-engine-py into main
pull from: issue/70-remove-sqlite-fallback-code-from-db-engine-py
merge into: cal:main
Conversation 0 Commits 1 Files Changed 1 +16 -26
Claude commented 2026-03-27 06:34:12 +00:00
Collaborator
Copy Link

Closes #70

Summary

Removes the DATABASE_TYPE conditional that allowed falling back to SQLite. PostgreSQL is now the only supported backend, consistent with project policy.

Changes

  • Removed DATABASE_TYPE = os.environ.get('DATABASE_TYPE', 'sqlite') and the entire if/else branch
  • Moved from playhouse.pool import PooledPostgresqlDatabase to top-level imports
  • Added _postgres_password check: raises RuntimeError('POSTGRES_PASSWORD environment variable is required') at startup if the env var is unset — prevents silent misconnection with misleading errors
  • Removed hardcoded 'sba_dev_password_2024' fallback password (same credential removed by PR #84 — included here since #84 is not yet merged; one of these will need a rebase)

Files Changed

  • app/db_engine.py — 27 lines removed, 17 lines added (net -10)

Other observations

  • PR #84 addresses the same hardcoded password removal. Whichever merges second will need a trivial rebase.
  • timeout=0 and autoconnect=True remain as-is (PR #87 addresses these — out of scope here).
Closes #70 ## Summary Removes the `DATABASE_TYPE` conditional that allowed falling back to SQLite. PostgreSQL is now the only supported backend, consistent with project policy. ## Changes - Removed `DATABASE_TYPE = os.environ.get('DATABASE_TYPE', 'sqlite')` and the entire if/else branch - Moved `from playhouse.pool import PooledPostgresqlDatabase` to top-level imports - Added `_postgres_password` check: raises `RuntimeError('POSTGRES_PASSWORD environment variable is required')` at startup if the env var is unset — prevents silent misconnection with misleading errors - Removed hardcoded `'sba_dev_password_2024'` fallback password (same credential removed by PR #84 — included here since #84 is not yet merged; one of these will need a rebase) ## Files Changed - `app/db_engine.py` — 27 lines removed, 17 lines added (net -10) ## Other observations - PR #84 addresses the same hardcoded password removal. Whichever merges second will need a trivial rebase. - `timeout=0` and `autoconnect=True` remain as-is (PR #87 addresses these — out of scope here).
Claude added 1 commit 2026-03-27 06:34:13 +00:00
fix: remove SQLite fallback code from db_engine.py (#70)
All checks were successful
Build Docker Image / build (pull_request) Successful in 2m46s
Details
e79623bd0b 
Removes DATABASE_TYPE conditional entirely. PostgreSQL is now the only
supported backend. Moves PooledPostgresqlDatabase import to top-level
and raises RuntimeError at startup if POSTGRES_PASSWORD is unset,
preventing silent misconnection with misleading errors.

Closes #70

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Claude added the
ai-reviewing
 label 2026-03-27 06:45:54 +00:00
Claude reviewed 2026-03-27 06:46:44 +00:00
Claude left a comment
Author
Collaborator
Copy Link

Review: APPROVED

Clean, correct removal of dead SQLite fallback code. PostgreSQL-only is consistent with project policy and all existing deployments.

What the PR does

  • Removes DATABASE_TYPE env var and the entire if/else branch — SQLite path was never used in production
  • Moves from playhouse.pool import PooledPostgresqlDatabase to top-level imports (correct per CLAUDE.md: no lazy imports in middle of file)
  • Adds _postgres_password is None → RuntimeError at startup — matches PR #84's approach exactly

Checks

Correctness: PooledPostgresqlDatabase constructor call is identical to the old PostgreSQL branch — no behavioural change for existing deployments.

Import ordering: from playhouse.pool import PooledPostgresqlDatabase is correctly placed between from peewee import ModelSelect and from playhouse.shortcuts import model_to_dict — alphabetically consistent.

Password guard: _postgres_password is None check is correct. Same edge case as PR #84: POSTGRES_PASSWORD="" passes the is None check and forwards an empty string to PostgreSQL → connection-time failure. Acceptable in practice (empty password is not a valid config).

SqliteDatabase symbol: Still available via from peewee import * but now unused. Not a problem — wildcard import, no action needed.

Rebase notes

This PR overlaps with two already-approved PRs:

  • PR #84 (hardcoded password removal) — shares the _postgres_password / RuntimeError pattern. Whichever merges second will have a trivial conflict at lines 14–21 of db_engine.py. One-line rebase.
  • PR #87 (autoconnect/timeout fixes) — changes timeout=0 → timeout=5 and autoconnect=True → autoconnect=False inside the same PooledPostgresqlDatabase(...) block. If #87 merges first, this PR will need those two parameter values updated before merge.

Merge order recommendation: #84 → #89 → #87 (or #89 → #84 → #87). Either way, #87 should go last since it modifies the constructor parameters that now live in the unconditional block introduced here.

No issues found

LGTM. ✓

## Review: APPROVED Clean, correct removal of dead SQLite fallback code. PostgreSQL-only is consistent with project policy and all existing deployments. ### What the PR does - Removes `DATABASE_TYPE` env var and the entire `if/else` branch — SQLite path was never used in production - Moves `from playhouse.pool import PooledPostgresqlDatabase` to top-level imports (correct per CLAUDE.md: no lazy imports in middle of file) - Adds `_postgres_password is None` → `RuntimeError` at startup — matches PR #84's approach exactly ### Checks **Correctness**: `PooledPostgresqlDatabase` constructor call is identical to the old PostgreSQL branch — no behavioural change for existing deployments. **Import ordering**: `from playhouse.pool import PooledPostgresqlDatabase` is correctly placed between `from peewee import ModelSelect` and `from playhouse.shortcuts import model_to_dict` — alphabetically consistent. **Password guard**: `_postgres_password is None` check is correct. Same edge case as PR #84: `POSTGRES_PASSWORD=""` passes the `is None` check and forwards an empty string to PostgreSQL → connection-time failure. Acceptable in practice (empty password is not a valid config). **`SqliteDatabase` symbol**: Still available via `from peewee import *` but now unused. Not a problem — wildcard import, no action needed. ### Rebase notes This PR overlaps with two already-approved PRs: - **PR #84** (hardcoded password removal) — shares the `_postgres_password` / `RuntimeError` pattern. Whichever merges second will have a trivial conflict at lines 14–21 of `db_engine.py`. One-line rebase. - **PR #87** (autoconnect/timeout fixes) — changes `timeout=0` → `timeout=5` and `autoconnect=True` → `autoconnect=False` inside the same `PooledPostgresqlDatabase(...)` block. If #87 merges first, this PR will need those two parameter values updated before merge. Merge order recommendation: **#84 → #89 → #87** (or #89 → #84 → #87). Either way, #87 should go last since it modifies the constructor parameters that now live in the unconditional block introduced here. ### No issues found LGTM. ✓
All checks were successful
Build Docker Image / build (pull_request) Successful in 2m46s
Details
This pull request has changes conflicting with the target branch.
  • app/db_engine.py
View command line instructions.

Checkout

From your project repository, check out a new branch and test the changes.
git fetch -u origin issue/70-remove-sqlite-fallback-code-from-db-engine-py:issue/70-remove-sqlite-fallback-code-from-db-engine-py
git checkout issue/70-remove-sqlite-fallback-code-from-db-engine-py
Sign in to join this conversation.
Reviewers
No reviewers
Labels
Clear labels   
ai-changes-requested

   
ai-pr-opened

   
ai-reviewed

   
ai-reviewing

   
ai-reviewing

   
ai-working
No Label
ai-changes-requested
ai-pr-opened
ai-reviewed
ai-reviewing
ai-reviewing
ai-working
Milestone
Clear milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
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/major-domo-database#89
No description provided.
Delete Branch "issue/70-remove-sqlite-fallback-code-from-db-engine-py"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?