refactor(config_store): replace generic key/value table with explicit schema

Switch orchestrator_config from a generic key/text store to a typed
single-row table with a dedicated teardown_timeout_seconds REAL column.
The CHECK (id = 1) constraint enforces at most one config row.

Remove TEARDOWN_TIMEOUT_KEY constant; update OrchestratorConfigStore to
expose get/set/delete_teardown_timeout_seconds() instead of the generic
get/set/delete(key). Resolving the DB value in resolve_teardown_timeout()
no longer needs a float() conversion since the column is already REAL.

bot_bottle/orchestrator/config_store.py

@@ -1,4 +1,4 @@
-"""Per-host orchestrator configuration store (key/value settings in bot-bottle.db).
+"""Per-host orchestrator configuration store (settings in bot-bottle.db).
 
 Co-tenants the shared `bot-bottle.db` via the `DbStore` framework.  Settings
 are readable by the host launch path directly (no HTTP round-trip to the
@@ -15,7 +15,6 @@ from ..db_store import DbStore
 from ..migrations import TableMigrations
 from ..paths import host_db_path
 
-TEARDOWN_TIMEOUT_KEY = "teardown_timeout_seconds"
 TEARDOWN_TIMEOUT_ENV = "BOT_BOTTLE_ORCHESTRATOR_TEARDOWN_TIMEOUT_SECONDS"
 DEFAULT_TEARDOWN_TIMEOUT_SECONDS = 30.0
 
@@ -24,8 +23,8 @@ _MIGRATIONS = TableMigrations(
     [
         """
         CREATE TABLE IF NOT EXISTS orchestrator_config (
-            key   TEXT PRIMARY KEY,
+            id                       INTEGER PRIMARY KEY CHECK (id = 1),
-            value TEXT NOT NULL
+            teardown_timeout_seconds REAL
         )
         """,
     ],
@@ -33,7 +32,7 @@ _MIGRATIONS = TableMigrations(
 
 
 class OrchestratorConfigStore(DbStore):
-    """Key/value store for orchestrator settings in the shared host DB."""
+    """Orchestrator settings in the shared host DB."""
 
     def __init__(self, db_path: Path | None = None) -> None:
         super().__init__(db_path or host_db_path(), _MIGRATIONS)
@@ -43,31 +42,33 @@ class OrchestratorConfigStore(DbStore):
         conn.execute("PRAGMA busy_timeout=5000")
         return conn
 
-    def get(self, key: str) -> str | None:
+    def get_teardown_timeout_seconds(self) -> float | None:
-        """Return the value for `key`, or None if absent."""
+        """Return the configured teardown timeout, or None if not set."""
         try:
             with self._connection() as conn:
                 row = conn.execute(
-                    "SELECT value FROM orchestrator_config WHERE key = ?", (key,)
+                    "SELECT teardown_timeout_seconds FROM orchestrator_config WHERE id = 1"
                 ).fetchone()
         except sqlite3.OperationalError:
             return None
-        return row["value"] if row else None
+        return row["teardown_timeout_seconds"] if row else None
 
-    def set(self, key: str, value: str) -> None:
+    def set_teardown_timeout_seconds(self, value: float) -> None:
-        """Upsert a setting."""
+        """Persist the teardown timeout."""
         with self._connection() as conn:
             conn.execute(
-                "INSERT OR REPLACE INTO orchestrator_config (key, value) VALUES (?, ?)",
+                "INSERT OR REPLACE INTO orchestrator_config"
-                (key, value),
+                " (id, teardown_timeout_seconds) VALUES (1, ?)",
+                (value,),
             )
         self._chmod()
 
-    def delete(self, key: str) -> bool:
+    def delete_teardown_timeout_seconds(self) -> bool:
-        """Remove a setting. Returns True if the key existed."""
+        """Clear the stored teardown timeout. Returns True if a value existed."""
         with self._connection() as conn:
             cur = conn.execute(
-                "DELETE FROM orchestrator_config WHERE key = ?", (key,)
+                "UPDATE orchestrator_config SET teardown_timeout_seconds = NULL"
+                " WHERE id = 1 AND teardown_timeout_seconds IS NOT NULL"
             )
         return cur.rowcount > 0
 
@@ -76,7 +77,7 @@ def resolve_teardown_timeout(db_path: Path | None = None) -> float:
     """Return the teardown timeout to use, in priority order:
 
     1. ``BOT_BOTTLE_ORCHESTRATOR_TEARDOWN_TIMEOUT_SECONDS`` env var
-    2. ``teardown_timeout_seconds`` key in the orchestrator config DB
+    2. ``teardown_timeout_seconds`` in the orchestrator config DB
     3. ``DEFAULT_TEARDOWN_TIMEOUT_SECONDS`` (30 s)
     """
     raw = os.environ.get(TEARDOWN_TIMEOUT_ENV, "").strip()
@@ -91,14 +92,9 @@ def resolve_teardown_timeout(db_path: Path | None = None) -> float:
     store = OrchestratorConfigStore(db_path)
     if not store.is_migrated():
         store.migrate()
-    raw_db = store.get(TEARDOWN_TIMEOUT_KEY)
+    db_value = store.get_teardown_timeout_seconds()
-    if raw_db is not None:
+    if db_value is not None and db_value > 0:
-        try:
+        return db_value
-            value = float(raw_db)
-            if value > 0:
-                return value
-        except ValueError:
-            pass
 
     return DEFAULT_TEARDOWN_TIMEOUT_SECONDS
 
@@ -106,7 +102,6 @@ def resolve_teardown_timeout(db_path: Path | None = None) -> float:
 __all__ = [
     "OrchestratorConfigStore",
     "resolve_teardown_timeout",
-    "TEARDOWN_TIMEOUT_KEY",
     "TEARDOWN_TIMEOUT_ENV",
     "DEFAULT_TEARDOWN_TIMEOUT_SECONDS",
 ]

tests/unit/test_orchestrator_config_store.py

@@ -18,7 +18,6 @@ from types import ModuleType
 from bot_bottle.orchestrator.config_store import (
     DEFAULT_TEARDOWN_TIMEOUT_SECONDS,
     TEARDOWN_TIMEOUT_ENV,
-    TEARDOWN_TIMEOUT_KEY,
     OrchestratorConfigStore,
     resolve_teardown_timeout,
 )
@@ -34,26 +33,26 @@ class TestOrchestratorConfigStore(unittest.TestCase):
     def tearDown(self) -> None:
         self._tmp.cleanup()
 
-    def test_get_returns_none_for_absent_key(self) -> None:
+    def test_get_returns_none_when_not_set(self) -> None:
-        self.assertIsNone(self.store.get("missing"))
+        self.assertIsNone(self.store.get_teardown_timeout_seconds())
 
     def test_set_and_get_roundtrip(self) -> None:
-        self.store.set("foo", "bar")
+        self.store.set_teardown_timeout_seconds(42.5)
-        self.assertEqual("bar", self.store.get("foo"))
+        self.assertEqual(42.5, self.store.get_teardown_timeout_seconds())
 
-    def test_set_upserts_existing_key(self) -> None:
+    def test_set_overwrites_existing_value(self) -> None:
-        self.store.set("k", "v1")
+        self.store.set_teardown_timeout_seconds(10.0)
-        self.store.set("k", "v2")
+        self.store.set_teardown_timeout_seconds(20.0)
-        self.assertEqual("v2", self.store.get("k"))
+        self.assertEqual(20.0, self.store.get_teardown_timeout_seconds())
 
-    def test_delete_removes_key(self) -> None:
+    def test_delete_clears_value_and_returns_true(self) -> None:
-        self.store.set("k", "v")
+        self.store.set_teardown_timeout_seconds(30.0)
-        deleted = self.store.delete("k")
+        deleted = self.store.delete_teardown_timeout_seconds()
         self.assertTrue(deleted)
-        self.assertIsNone(self.store.get("k"))
+        self.assertIsNone(self.store.get_teardown_timeout_seconds())
 
     def test_delete_absent_returns_false(self) -> None:
-        self.assertFalse(self.store.delete("nope"))
+        self.assertFalse(self.store.delete_teardown_timeout_seconds())
 
     def test_is_migrated_true_after_migrate(self) -> None:
         self.assertTrue(self.store.is_migrated())
@@ -85,14 +84,14 @@ class TestResolveTeardownTimeout(unittest.TestCase):
     def test_env_var_overrides_db_value(self) -> None:
         store = OrchestratorConfigStore(self.db)
         store.migrate()
-        store.set(TEARDOWN_TIMEOUT_KEY, "55")
+        store.set_teardown_timeout_seconds(55.0)
         os.environ[TEARDOWN_TIMEOUT_ENV] = "77"
         self.assertEqual(77.0, resolve_teardown_timeout(self.db))
 
     def test_db_value_overrides_default(self) -> None:
         store = OrchestratorConfigStore(self.db)
         store.migrate()
-        store.set(TEARDOWN_TIMEOUT_KEY, "42")
+        store.set_teardown_timeout_seconds(42.0)
         self.assertEqual(42.0, resolve_teardown_timeout(self.db))
 
     def test_invalid_env_var_falls_through_to_default(self) -> None:
@@ -103,14 +102,13 @@ class TestResolveTeardownTimeout(unittest.TestCase):
         os.environ[TEARDOWN_TIMEOUT_ENV] = "0"
         self.assertEqual(DEFAULT_TEARDOWN_TIMEOUT_SECONDS, resolve_teardown_timeout(self.db))
 
-    def test_invalid_db_value_falls_through_to_default(self) -> None:
+    def test_non_positive_db_value_falls_through_to_default(self) -> None:
         store = OrchestratorConfigStore(self.db)
         store.migrate()
-        store.set(TEARDOWN_TIMEOUT_KEY, "bad")
+        store.set_teardown_timeout_seconds(0.0)
         self.assertEqual(DEFAULT_TEARDOWN_TIMEOUT_SECONDS, resolve_teardown_timeout(self.db))
 
     def test_migrates_db_on_first_call(self) -> None:
-        # DB does not exist yet; resolve_teardown_timeout must not raise.
         result = resolve_teardown_timeout(self.db)
         self.assertEqual(DEFAULT_TEARDOWN_TIMEOUT_SECONDS, result)
         self.assertTrue(OrchestratorConfigStore(self.db).is_migrated())