From a4d82e5ff2ff41da7d2dc56867ad986fe9c6e249 Mon Sep 17 00:00:00 2001 From: claude Date: Thu, 2 Jul 2026 21:04:34 +0000 Subject: [PATCH] refactor: extract TableMigrations and DbStore base class Adds bot_bottle/migrations.py (TableMigrations) and bot_bottle/db_store.py (DbStore) per PR review. Both stores now inherit from DbStore and hold a TableMigrations instance instead of duplicating schema-version logic inline. --- Dockerfile.sidecars | 2 ++ bot_bottle/audit_store.py | 60 ++++++++++--------------------------- bot_bottle/db_store.py | 40 +++++++++++++++++++++++++ bot_bottle/migrations.py | 37 +++++++++++++++++++++++ bot_bottle/queue_store.py | 62 ++++++++++----------------------------- 5 files changed, 110 insertions(+), 91 deletions(-) create mode 100644 bot_bottle/db_store.py create mode 100644 bot_bottle/migrations.py diff --git a/Dockerfile.sidecars b/Dockerfile.sidecars index 2b43ee1..2e28b1a 100644 --- a/Dockerfile.sidecars +++ b/Dockerfile.sidecars @@ -66,6 +66,8 @@ COPY bot_bottle/egress_dlp_config.py /app/egress_dlp_config.py COPY bot_bottle/egress_addon.py /app/egress_addon.py COPY bot_bottle/dlp_detectors.py /app/dlp_detectors.py COPY bot_bottle/yaml_subset.py /app/yaml_subset.py +COPY bot_bottle/migrations.py /app/migrations.py +COPY bot_bottle/db_store.py /app/db_store.py COPY bot_bottle/queue_store.py /app/queue_store.py COPY bot_bottle/audit_store.py /app/audit_store.py COPY bot_bottle/supervise.py /app/supervise.py diff --git a/bot_bottle/audit_store.py b/bot_bottle/audit_store.py index 9d5518d..5f848d0 100644 --- a/bot_bottle/audit_store.py +++ b/bot_bottle/audit_store.py @@ -9,6 +9,13 @@ from typing import TYPE_CHECKING if TYPE_CHECKING: from .supervise import AuditEntry +try: + from .db_store import DbStore + from .migrations import TableMigrations +except ImportError: + from db_store import DbStore # type: ignore[import-not-found] # pylint: disable=import-error,no-name-in-module + from migrations import TableMigrations # type: ignore[import-not-found] # pylint: disable=import-error,no-name-in-module + def get_supervise_mod() -> object: """Lazy import of supervise to avoid a circular-import at module init time. @@ -26,10 +33,10 @@ def get_supervise_mod() -> object: return _m -# One entry per schema version: _MIGRATIONS[0] brings a fresh DB (user_version=0) -# to version 1, _MIGRATIONS[1] to version 2, and so on. Add new migrations at -# the end; never edit existing ones. -_MIGRATIONS: list[str] = [ +# One entry per schema version: _MIGRATIONS.migrations[0] brings a fresh DB +# to version 1, [1] to version 2, and so on. Add new migrations at the end; +# never edit existing ones. +_MIGRATIONS = TableMigrations("audit_store", [ # v1 — initial schema """ CREATE TABLE IF NOT EXISTS supervise_audit_entries ( @@ -43,16 +50,15 @@ _MIGRATIONS: list[str] = [ diff TEXT NOT NULL ) """, -] +]) -class AuditStore: +class AuditStore(DbStore): """SQLite-backed persistent store for supervise audit entries.""" def __init__(self, db_path: Path | None = None) -> None: - self.db_path = db_path or get_supervise_mod().host_db_path() # type: ignore[attr-defined] - self.db_path.parent.mkdir(parents=True, exist_ok=True) - self._init() + resolved = db_path or get_supervise_mod().host_db_path() # type: ignore[attr-defined] + super().__init__(resolved, _MIGRATIONS) def write_audit_entry(self, entry: AuditEntry) -> Path: with self._connect() as conn: @@ -103,41 +109,5 @@ class AuditStore: diff=row["diff"], ) - def _connect(self) -> sqlite3.Connection: - conn = sqlite3.connect(self.db_path) - conn.row_factory = sqlite3.Row - return conn - - _SCHEMA_KEY = "audit_store" - - def _init(self) -> None: - with self._connect() as conn: - conn.execute( - """ - CREATE TABLE IF NOT EXISTS schema_versions ( - module TEXT PRIMARY KEY, - version INTEGER NOT NULL DEFAULT 0 - ) - """ - ) - row = conn.execute( - "SELECT version FROM schema_versions WHERE module = ?", - (self._SCHEMA_KEY,), - ).fetchone() - version = row[0] if row else 0 - for i, sql in enumerate(_MIGRATIONS[version:], start=version + 1): - conn.execute(sql) - conn.execute( - "INSERT OR REPLACE INTO schema_versions (module, version) VALUES (?, ?)", - (self._SCHEMA_KEY, i), - ) - self._chmod() - - def _chmod(self) -> None: - try: - self.db_path.chmod(0o600) - except OSError: - pass - __all__ = ["AuditStore"] diff --git a/bot_bottle/db_store.py b/bot_bottle/db_store.py new file mode 100644 index 0000000..7cc1849 --- /dev/null +++ b/bot_bottle/db_store.py @@ -0,0 +1,40 @@ +"""Shared SQLite-backed store base class for bot-bottle (PRD 0013).""" + +from __future__ import annotations + +import sqlite3 +from pathlib import Path + +try: + from .migrations import TableMigrations +except ImportError: + from migrations import TableMigrations # type: ignore[import-not-found] # pylint: disable=import-error,no-name-in-module + + +class DbStore: + """Base for SQLite-backed stores. Subclasses resolve db_path then call super().__init__.""" + + def __init__(self, db_path: Path, migrations: TableMigrations) -> None: + self.db_path = db_path + self._migrations = migrations + self.db_path.parent.mkdir(parents=True, exist_ok=True) + self._init() + + def _connect(self) -> sqlite3.Connection: + conn = sqlite3.connect(self.db_path) + conn.row_factory = sqlite3.Row + return conn + + def _init(self) -> None: + with self._connect() as conn: + self._migrations.apply(conn) + self._chmod() + + def _chmod(self) -> None: + try: + self.db_path.chmod(0o600) + except OSError: + pass + + +__all__ = ["DbStore"] diff --git a/bot_bottle/migrations.py b/bot_bottle/migrations.py new file mode 100644 index 0000000..83fccf6 --- /dev/null +++ b/bot_bottle/migrations.py @@ -0,0 +1,37 @@ +"""SQLite migration runner for bot-bottle stores.""" + +from __future__ import annotations + +import sqlite3 + + +class TableMigrations: + """Runs a sequential list of DDL migrations tracked by schema_key in schema_versions.""" + + def __init__(self, schema_key: str, migrations: list[str]) -> None: + self.schema_key = schema_key + self.migrations = migrations + + def apply(self, conn: sqlite3.Connection) -> None: + conn.execute( + """ + CREATE TABLE IF NOT EXISTS schema_versions ( + module TEXT PRIMARY KEY, + version INTEGER NOT NULL DEFAULT 0 + ) + """ + ) + row = conn.execute( + "SELECT version FROM schema_versions WHERE module = ?", + (self.schema_key,), + ).fetchone() + version = row[0] if row else 0 + for i, sql in enumerate(self.migrations[version:], start=version + 1): + conn.execute(sql) + conn.execute( + "INSERT OR REPLACE INTO schema_versions (module, version) VALUES (?, ?)", + (self.schema_key, i), + ) + + +__all__ = ["TableMigrations"] diff --git a/bot_bottle/queue_store.py b/bot_bottle/queue_store.py index 6ef5dcb..76a02da 100644 --- a/bot_bottle/queue_store.py +++ b/bot_bottle/queue_store.py @@ -10,6 +10,13 @@ from typing import TYPE_CHECKING if TYPE_CHECKING: from .supervise import Proposal, Response +try: + from .db_store import DbStore + from .migrations import TableMigrations +except ImportError: + from db_store import DbStore # type: ignore[import-not-found] # pylint: disable=import-error,no-name-in-module + from migrations import TableMigrations # type: ignore[import-not-found] # pylint: disable=import-error,no-name-in-module + def get_supervise_mod() -> object: """Lazy import of supervise to avoid a circular-import at module init time. @@ -30,10 +37,10 @@ def get_supervise_mod() -> object: return _m -# One entry per schema version: _MIGRATIONS[0] brings a fresh DB (user_version=0) -# to version 1, _MIGRATIONS[1] to version 2, and so on. Add new migrations at -# the end; never edit existing ones. -_MIGRATIONS: list[str] = [ +# One entry per schema version: _MIGRATIONS.migrations[0] brings a fresh DB +# to version 1, [1] to version 2, and so on. Add new migrations at the end; +# never edit existing ones. +_MIGRATIONS = TableMigrations("queue_store", [ # v1 — proposals table """ CREATE TABLE IF NOT EXISTS supervise_proposals ( @@ -61,24 +68,23 @@ _MIGRATIONS: list[str] = [ PRIMARY KEY (queue_key, proposal_id) ) """, -] +]) -class QueueStore: +class QueueStore(DbStore): """SQLite-backed persistent store for supervise proposals and responses.""" def __init__(self, queue_key: str, db_path: Path | None = None) -> None: self.queue_key = queue_key if db_path is not None: - self.db_path = db_path + resolved = db_path else: # In the sidecar container SUPERVISE_DB_PATH points at the # bind-mounted host DB. On the host this env var is never set, # so we always fall through to host_db_path(). env_path = os.environ.get("SUPERVISE_DB_PATH", "").strip() - self.db_path = Path(env_path) if env_path else get_supervise_mod().host_db_path() # type: ignore[attr-defined] - self.db_path.parent.mkdir(parents=True, exist_ok=True) - self._init() + resolved = Path(env_path) if env_path else get_supervise_mod().host_db_path() # type: ignore[attr-defined] + super().__init__(resolved, _MIGRATIONS) def write_proposal(self, proposal: Proposal) -> Path: with self._connect() as conn: @@ -230,41 +236,5 @@ class QueueStore: final_file=row["final_file"], ) - def _connect(self) -> sqlite3.Connection: - conn = sqlite3.connect(self.db_path) - conn.row_factory = sqlite3.Row - return conn - - _SCHEMA_KEY = "queue_store" - - def _init(self) -> None: - with self._connect() as conn: - conn.execute( - """ - CREATE TABLE IF NOT EXISTS schema_versions ( - module TEXT PRIMARY KEY, - version INTEGER NOT NULL DEFAULT 0 - ) - """ - ) - row = conn.execute( - "SELECT version FROM schema_versions WHERE module = ?", - (self._SCHEMA_KEY,), - ).fetchone() - version = row[0] if row else 0 - for i, sql in enumerate(_MIGRATIONS[version:], start=version + 1): - conn.execute(sql) - conn.execute( - "INSERT OR REPLACE INTO schema_versions (module, version) VALUES (?, ?)", - (self._SCHEMA_KEY, i), - ) - self._chmod() - - def _chmod(self) -> None: - try: - self.db_path.chmod(0o600) - except OSError: - pass - __all__ = ["QueueStore"]