Opt database bootstrapping
This commit is contained in:
@@ -12,6 +12,7 @@
|
||||
- The CLI root now lazy-loads heavy runtime dependencies so lightweight commands such as `version`, `help`, `configure_workstation`, and `upgrade` stay import-light.
|
||||
- Shared CLI defaults for container/output tokens now live outside [`src/ffx/ffx_controller.py`](/home/osgw/.local/src/codex/ffx/src/ffx/ffx_controller.py), and a focused unit test locks in the lazy-import contract.
|
||||
- `FileProperties` now uses one cached `ffprobe -show_format -show_streams -of json` call per source file, and the combined payload was confirmed against the Dragonball asset to satisfy both previous probe call sites fully.
|
||||
- Database startup now bootstraps schema only when required tables are actually missing, while version enforcement still runs on ordinary DB-backed context creation.
|
||||
- FFX logger setup now reuses named handlers, and fallback logger access no longer mutates handlers in ordinary constructors and helpers.
|
||||
- The process wrapper now uses `subprocess.run(...)` with centralized command formatting plus stable timeout and missing-command error mapping.
|
||||
- Active ORM controllers now use single-query accessors instead of paired `count()` plus `first()` lookups.
|
||||
@@ -103,15 +104,6 @@
|
||||
- Cleaner runtime output.
|
||||
- Less warning noise during dry-run maintenance commands.
|
||||
|
||||
9. Database startup always runs schema creation and version checks
|
||||
- [`src/ffx/database.py`](/home/osgw/.local/src/codex/ffx/src/ffx/database.py) runs `Base.metadata.create_all(...)` and version checks every time a DB-backed context is created.
|
||||
- Optimization:
|
||||
- Measure startup cost and consider separating bootstrapping from ordinary command execution.
|
||||
- Keep schema migration/version enforcement explicit.
|
||||
- Expected value:
|
||||
- Faster command startup.
|
||||
- Clearer operational boundaries.
|
||||
|
||||
## Open
|
||||
|
||||
- Should optimization work focus first on operator-perceived latency, internal maintainability, or correctness-risk cleanup that also has performance upside?
|
||||
|
||||
@@ -1,6 +1,6 @@
|
||||
import os, click
|
||||
|
||||
from sqlalchemy import create_engine
|
||||
from sqlalchemy import create_engine, inspect
|
||||
from sqlalchemy.orm import sessionmaker
|
||||
|
||||
# Import the full model package so SQLAlchemy registers every mapped class
|
||||
@@ -14,6 +14,7 @@ from ffx.constants import DATABASE_VERSION
|
||||
|
||||
|
||||
DATABASE_VERSION_KEY = 'database_version'
|
||||
EXPECTED_TABLE_NAMES = set(Base.metadata.tables.keys())
|
||||
|
||||
class DatabaseVersionException(Exception):
|
||||
def __init__(self, errorMessage):
|
||||
@@ -37,7 +38,7 @@ def databaseContext(databasePath: str = ''):
|
||||
databaseContext['engine'] = create_engine(databaseContext['url'])
|
||||
databaseContext['session'] = sessionmaker(bind=databaseContext['engine'])
|
||||
|
||||
Base.metadata.create_all(databaseContext['engine'])
|
||||
bootstrapDatabaseIfNeeded(databaseContext)
|
||||
|
||||
# isSyncronuous = False
|
||||
# while not isSyncronuous:
|
||||
@@ -54,6 +55,19 @@ def databaseContext(databasePath: str = ''):
|
||||
|
||||
return databaseContext
|
||||
|
||||
|
||||
def databaseNeedsBootstrap(databaseContext) -> bool:
|
||||
inspector = inspect(databaseContext['engine'])
|
||||
existingTableNames = set(inspector.get_table_names())
|
||||
return not EXPECTED_TABLE_NAMES.issubset(existingTableNames)
|
||||
|
||||
|
||||
def bootstrapDatabaseIfNeeded(databaseContext):
|
||||
if not databaseNeedsBootstrap(databaseContext):
|
||||
return
|
||||
|
||||
Base.metadata.create_all(databaseContext['engine'])
|
||||
|
||||
def ensureDatabaseVersion(databaseContext):
|
||||
|
||||
currentDatabaseVersion = getDatabaseVersion(databaseContext)
|
||||
|
||||
83
tests/unit/test_database.py
Normal file
83
tests/unit/test_database.py
Normal file
@@ -0,0 +1,83 @@
|
||||
from __future__ import annotations
|
||||
|
||||
from pathlib import Path
|
||||
import sys
|
||||
import tempfile
|
||||
import unittest
|
||||
from unittest.mock import patch
|
||||
|
||||
|
||||
SRC_ROOT = Path(__file__).resolve().parents[2] / "src"
|
||||
|
||||
if str(SRC_ROOT) not in sys.path:
|
||||
sys.path.insert(0, str(SRC_ROOT))
|
||||
|
||||
|
||||
from ffx.constants import DATABASE_VERSION # noqa: E402
|
||||
from ffx.database import DATABASE_VERSION_KEY, databaseContext, getDatabaseVersion # noqa: E402
|
||||
from ffx.model.property import Property # noqa: E402
|
||||
from ffx.model.show import Base # noqa: E402
|
||||
|
||||
|
||||
class DatabaseContextTests(unittest.TestCase):
|
||||
def setUp(self):
|
||||
self.tempdir = tempfile.TemporaryDirectory()
|
||||
self.database_path = Path(self.tempdir.name) / "ffx-test.db"
|
||||
|
||||
def tearDown(self):
|
||||
self.tempdir.cleanup()
|
||||
|
||||
def test_database_context_bootstraps_new_database_with_current_version(self):
|
||||
with patch("ffx.database.Base.metadata.create_all", wraps=Base.metadata.create_all) as mocked_create_all:
|
||||
context = databaseContext(str(self.database_path))
|
||||
try:
|
||||
self.assertTrue(self.database_path.exists())
|
||||
self.assertEqual(DATABASE_VERSION, getDatabaseVersion(context))
|
||||
finally:
|
||||
context["engine"].dispose()
|
||||
|
||||
mocked_create_all.assert_called_once()
|
||||
|
||||
def test_database_context_skips_create_all_when_schema_is_already_present(self):
|
||||
initial_context = databaseContext(str(self.database_path))
|
||||
initial_context["engine"].dispose()
|
||||
|
||||
with patch("ffx.database.Base.metadata.create_all") as mocked_create_all:
|
||||
context = databaseContext(str(self.database_path))
|
||||
try:
|
||||
self.assertEqual(DATABASE_VERSION, getDatabaseVersion(context))
|
||||
finally:
|
||||
context["engine"].dispose()
|
||||
|
||||
mocked_create_all.assert_not_called()
|
||||
|
||||
def test_database_context_restores_missing_version_property_without_schema_bootstrap(self):
|
||||
context = databaseContext(str(self.database_path))
|
||||
Session = context["session"]
|
||||
try:
|
||||
session = Session()
|
||||
try:
|
||||
version_row = (
|
||||
session.query(Property)
|
||||
.filter(Property.key == DATABASE_VERSION_KEY)
|
||||
.first()
|
||||
)
|
||||
session.delete(version_row)
|
||||
session.commit()
|
||||
finally:
|
||||
session.close()
|
||||
finally:
|
||||
context["engine"].dispose()
|
||||
|
||||
with patch("ffx.database.Base.metadata.create_all") as mocked_create_all:
|
||||
reopened_context = databaseContext(str(self.database_path))
|
||||
try:
|
||||
self.assertEqual(DATABASE_VERSION, getDatabaseVersion(reopened_context))
|
||||
finally:
|
||||
reopened_context["engine"].dispose()
|
||||
|
||||
mocked_create_all.assert_not_called()
|
||||
|
||||
|
||||
if __name__ == "__main__":
|
||||
unittest.main()
|
||||
Reference in New Issue
Block a user