refactor(dry-B2): ImportSettings.load()/load_sync() classmethods for the singleton row
The `select(ImportSettings).where(id == 1)).scalar_one()` singleton load was repeated 15× across services, API, and 5 task modules. Added async load() + sync load_sync() classmethods on the model and migrated all 15 full-row sites (callers already imported ImportSettings, so no new imports; dropped download's now-orphaned select import). Left maintenance.py's deliberate column-select (import_scan_path only) as-is. Rest of the service layer was already adequately DRY — the Record/to_dict pattern is only 2 instances and the savepoint find-or-create recovery is correctly per-entity, so neither was forced into a shared abstraction. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -159,9 +159,7 @@ def _refetch_task_sync(session, task_id: int) -> dict:
|
|||||||
return {"status": "not_found"}
|
return {"status": "not_found"}
|
||||||
if task.status != "failed":
|
if task.status != "failed":
|
||||||
return {"status": "not_failed"}
|
return {"status": "not_failed"}
|
||||||
settings = session.execute(
|
settings = ImportSettings.load_sync(session)
|
||||||
select(ImportSettings).where(ImportSettings.id == 1)
|
|
||||||
).scalar_one()
|
|
||||||
return attempt_refetch(session, task, Path(settings.import_scan_path))
|
return attempt_refetch(session, task, Path(settings.import_scan_path))
|
||||||
|
|
||||||
|
|
||||||
|
|||||||
@@ -31,9 +31,7 @@ _EDITABLE_FIELDS = (
|
|||||||
@settings_bp.route("/settings/import", methods=["GET"])
|
@settings_bp.route("/settings/import", methods=["GET"])
|
||||||
async def get_import_settings():
|
async def get_import_settings():
|
||||||
async with get_session() as session:
|
async with get_session() as session:
|
||||||
row = (
|
row = await ImportSettings.load(session)
|
||||||
await session.execute(select(ImportSettings).where(ImportSettings.id == 1))
|
|
||||||
).scalar_one()
|
|
||||||
return jsonify({
|
return jsonify({
|
||||||
"min_width": row.min_width,
|
"min_width": row.min_width,
|
||||||
"min_height": row.min_height,
|
"min_height": row.min_height,
|
||||||
@@ -99,9 +97,7 @@ async def update_import_settings():
|
|||||||
return _bad_int("download_failure_warning_threshold", 1, 100)
|
return _bad_int("download_failure_warning_threshold", 1, 100)
|
||||||
|
|
||||||
async with get_session() as session:
|
async with get_session() as session:
|
||||||
row = (
|
row = await ImportSettings.load(session)
|
||||||
await session.execute(select(ImportSettings).where(ImportSettings.id == 1))
|
|
||||||
).scalar_one()
|
|
||||||
for field in _EDITABLE_FIELDS:
|
for field in _EDITABLE_FIELDS:
|
||||||
if field in body:
|
if field in body:
|
||||||
setattr(row, field, body[field])
|
setattr(row, field, body[field])
|
||||||
|
|||||||
@@ -227,9 +227,7 @@ async def delete_run(run_id: int):
|
|||||||
@system_backup_bp.route("/settings", methods=["GET"])
|
@system_backup_bp.route("/settings", methods=["GET"])
|
||||||
async def get_settings():
|
async def get_settings():
|
||||||
async with get_session() as session:
|
async with get_session() as session:
|
||||||
row = (await session.execute(
|
row = await ImportSettings.load(session)
|
||||||
select(ImportSettings).where(ImportSettings.id == 1)
|
|
||||||
)).scalar_one()
|
|
||||||
return jsonify({
|
return jsonify({
|
||||||
"backup_db_nightly_enabled": row.backup_db_nightly_enabled,
|
"backup_db_nightly_enabled": row.backup_db_nightly_enabled,
|
||||||
"backup_db_nightly_hour_utc": row.backup_db_nightly_hour_utc,
|
"backup_db_nightly_hour_utc": row.backup_db_nightly_hour_utc,
|
||||||
@@ -249,9 +247,7 @@ async def patch_settings():
|
|||||||
return err
|
return err
|
||||||
|
|
||||||
async with get_session() as session:
|
async with get_session() as session:
|
||||||
row = (await session.execute(
|
row = await ImportSettings.load(session)
|
||||||
select(ImportSettings).where(ImportSettings.id == 1)
|
|
||||||
)).scalar_one()
|
|
||||||
for field in _BACKUP_SETTINGS_FIELDS:
|
for field in _BACKUP_SETTINGS_FIELDS:
|
||||||
if field in body:
|
if field in body:
|
||||||
setattr(row, field, body[field])
|
setattr(row, field, body[field])
|
||||||
|
|||||||
@@ -4,7 +4,7 @@ Enforced as a single row via a CHECK (id = 1) constraint. The application
|
|||||||
always SELECTs id=1 and never inserts/deletes after the initial migration.
|
always SELECTs id=1 and never inserts/deletes after the initial migration.
|
||||||
"""
|
"""
|
||||||
|
|
||||||
from sqlalchemy import Boolean, CheckConstraint, Float, Integer, Text
|
from sqlalchemy import Boolean, CheckConstraint, Float, Integer, Text, select
|
||||||
from sqlalchemy.orm import Mapped, mapped_column
|
from sqlalchemy.orm import Mapped, mapped_column
|
||||||
|
|
||||||
from .base import Base
|
from .base import Base
|
||||||
@@ -63,3 +63,13 @@ class ImportSettings(Base):
|
|||||||
backup_images_keep_last_n: Mapped[int] = mapped_column(
|
backup_images_keep_last_n: Mapped[int] = mapped_column(
|
||||||
Integer, nullable=False, default=3,
|
Integer, nullable=False, default=3,
|
||||||
)
|
)
|
||||||
|
|
||||||
|
@classmethod
|
||||||
|
async def load(cls, session) -> "ImportSettings":
|
||||||
|
"""The singleton settings row (id=1), via an async session."""
|
||||||
|
return (await session.execute(select(cls).where(cls.id == 1))).scalar_one()
|
||||||
|
|
||||||
|
@classmethod
|
||||||
|
def load_sync(cls, session) -> "ImportSettings":
|
||||||
|
"""The singleton settings row (id=1), via a sync session."""
|
||||||
|
return session.execute(select(cls).where(cls.id == 1)).scalar_one()
|
||||||
|
|||||||
@@ -58,9 +58,7 @@ async def select_due_sources(session: AsyncSession) -> list[Source]:
|
|||||||
.where(Artist.auto_check.is_(True))
|
.where(Artist.auto_check.is_(True))
|
||||||
)).scalars().all()
|
)).scalars().all()
|
||||||
|
|
||||||
settings = (await session.execute(
|
settings = await ImportSettings.load(session)
|
||||||
select(ImportSettings).where(ImportSettings.id == 1)
|
|
||||||
)).scalar_one()
|
|
||||||
|
|
||||||
now = datetime.now(UTC)
|
now = datetime.now(UTC)
|
||||||
due: list[Source] = []
|
due: list[Source] = []
|
||||||
@@ -121,9 +119,7 @@ async def scheduler_status(session: AsyncSession) -> dict:
|
|||||||
.where(Source.enabled.is_(True))
|
.where(Source.enabled.is_(True))
|
||||||
.where(Artist.auto_check.is_(True))
|
.where(Artist.auto_check.is_(True))
|
||||||
)).scalars().all()
|
)).scalars().all()
|
||||||
settings = (await session.execute(
|
settings = await ImportSettings.load(session)
|
||||||
select(ImportSettings).where(ImportSettings.id == 1)
|
|
||||||
)).scalar_one()
|
|
||||||
|
|
||||||
now = datetime.now(UTC)
|
now = datetime.now(UTC)
|
||||||
due_now = 0
|
due_now = 0
|
||||||
|
|||||||
@@ -120,9 +120,7 @@ class SourceService:
|
|||||||
return config
|
return config
|
||||||
|
|
||||||
async def _load_settings(self) -> ImportSettings:
|
async def _load_settings(self) -> ImportSettings:
|
||||||
return (await self.session.execute(
|
return await ImportSettings.load(self.session)
|
||||||
select(ImportSettings).where(ImportSettings.id == 1)
|
|
||||||
)).scalar_one()
|
|
||||||
|
|
||||||
def _build_record(
|
def _build_record(
|
||||||
self, source: Source, artist: Artist, settings: ImportSettings,
|
self, source: Source, artist: Artist, settings: ImportSettings,
|
||||||
|
|||||||
@@ -246,9 +246,7 @@ def prune_backups() -> dict:
|
|||||||
SessionLocal = _sync_session_factory()
|
SessionLocal = _sync_session_factory()
|
||||||
counts = {"db_deleted": 0, "images_deleted": 0, "files_unlinked": 0}
|
counts = {"db_deleted": 0, "images_deleted": 0, "files_unlinked": 0}
|
||||||
with SessionLocal() as session:
|
with SessionLocal() as session:
|
||||||
s = session.execute(
|
s = ImportSettings.load_sync(session)
|
||||||
select(ImportSettings).where(ImportSettings.id == 1)
|
|
||||||
).scalar_one()
|
|
||||||
for kind, keep in (
|
for kind, keep in (
|
||||||
("db", s.backup_db_keep_last_n),
|
("db", s.backup_db_keep_last_n),
|
||||||
("images", s.backup_images_keep_last_n),
|
("images", s.backup_images_keep_last_n),
|
||||||
@@ -286,9 +284,7 @@ def backup_db_nightly() -> dict:
|
|||||||
either {'skipped': '<reason>'} or {'dispatched': '<task_id>'}."""
|
either {'skipped': '<reason>'} or {'dispatched': '<task_id>'}."""
|
||||||
SessionLocal = _sync_session_factory()
|
SessionLocal = _sync_session_factory()
|
||||||
with SessionLocal() as session:
|
with SessionLocal() as session:
|
||||||
s = session.execute(
|
s = ImportSettings.load_sync(session)
|
||||||
select(ImportSettings).where(ImportSettings.id == 1)
|
|
||||||
).scalar_one()
|
|
||||||
nightly_enabled = s.backup_db_nightly_enabled
|
nightly_enabled = s.backup_db_nightly_enabled
|
||||||
configured_hour = s.backup_db_nightly_hour_utc
|
configured_hour = s.backup_db_nightly_hour_utc
|
||||||
if not nightly_enabled:
|
if not nightly_enabled:
|
||||||
|
|||||||
@@ -3,7 +3,6 @@
|
|||||||
import asyncio
|
import asyncio
|
||||||
from pathlib import Path
|
from pathlib import Path
|
||||||
|
|
||||||
from sqlalchemy import select
|
|
||||||
from sqlalchemy.exc import DBAPIError, OperationalError
|
from sqlalchemy.exc import DBAPIError, OperationalError
|
||||||
|
|
||||||
from ..celery_app import celery
|
from ..celery_app import celery
|
||||||
@@ -41,9 +40,7 @@ def download_source(self, source_id: int) -> int:
|
|||||||
SyncFactory = _sync_session_factory()
|
SyncFactory = _sync_session_factory()
|
||||||
try:
|
try:
|
||||||
with SyncFactory() as sync_session:
|
with SyncFactory() as sync_session:
|
||||||
settings = sync_session.execute(
|
settings = ImportSettings.load_sync(sync_session)
|
||||||
select(ImportSettings).where(ImportSettings.id == 1)
|
|
||||||
).scalar_one()
|
|
||||||
rate_limit = settings.download_rate_limit_seconds
|
rate_limit = settings.download_rate_limit_seconds
|
||||||
validate_files = settings.download_validate_files
|
validate_files = settings.download_validate_files
|
||||||
|
|
||||||
@@ -57,9 +54,7 @@ def download_source(self, source_id: int) -> int:
|
|||||||
async with async_factory() as async_session:
|
async with async_factory() as async_session:
|
||||||
cred_service = CredentialService(async_session, crypto)
|
cred_service = CredentialService(async_session, crypto)
|
||||||
with SyncFactory() as sync_session:
|
with SyncFactory() as sync_session:
|
||||||
sync_settings = sync_session.execute(
|
sync_settings = ImportSettings.load_sync(sync_session)
|
||||||
select(ImportSettings).where(ImportSettings.id == 1)
|
|
||||||
).scalar_one()
|
|
||||||
importer = Importer(
|
importer = Importer(
|
||||||
session=sync_session,
|
session=sync_session,
|
||||||
images_root=IMAGES_ROOT,
|
images_root=IMAGES_ROOT,
|
||||||
|
|||||||
@@ -167,9 +167,7 @@ def enqueue_import(task_id: int, task_type: str) -> None:
|
|||||||
|
|
||||||
def _do_import(session, task, import_task_id: int) -> dict:
|
def _do_import(session, task, import_task_id: int) -> dict:
|
||||||
"""Actual work, called from inside the resilience wrapper."""
|
"""Actual work, called from inside the resilience wrapper."""
|
||||||
settings = session.execute(
|
settings = ImportSettings.load_sync(session)
|
||||||
select(ImportSettings).where(ImportSettings.id == 1)
|
|
||||||
).scalar_one()
|
|
||||||
import_root = Path(settings.import_scan_path)
|
import_root = Path(settings.import_scan_path)
|
||||||
batch = session.get(ImportBatch, task.batch_id)
|
batch = session.get(ImportBatch, task.batch_id)
|
||||||
deep = bool(batch and batch.scan_mode == "deep")
|
deep = bool(batch and batch.scan_mode == "deep")
|
||||||
|
|||||||
@@ -460,9 +460,7 @@ def cleanup_old_download_events() -> int:
|
|||||||
"""
|
"""
|
||||||
SessionLocal = _sync_session_factory()
|
SessionLocal = _sync_session_factory()
|
||||||
with SessionLocal() as session:
|
with SessionLocal() as session:
|
||||||
settings = session.execute(
|
settings = ImportSettings.load_sync(session)
|
||||||
select(ImportSettings).where(ImportSettings.id == 1)
|
|
||||||
).scalar_one()
|
|
||||||
retention_days = settings.download_event_retention_days
|
retention_days = settings.download_event_retention_days
|
||||||
cutoff = datetime.now(UTC) - timedelta(days=retention_days)
|
cutoff = datetime.now(UTC) - timedelta(days=retention_days)
|
||||||
result = session.execute(
|
result = session.execute(
|
||||||
|
|||||||
@@ -44,9 +44,7 @@ def scan_directory(self, triggered_by: str = "manual",
|
|||||||
batch id."""
|
batch id."""
|
||||||
SessionLocal = _sync_session_factory()
|
SessionLocal = _sync_session_factory()
|
||||||
with SessionLocal() as session:
|
with SessionLocal() as session:
|
||||||
settings = session.execute(
|
settings = ImportSettings.load_sync(session)
|
||||||
select(ImportSettings).where(ImportSettings.id == 1)
|
|
||||||
).scalar_one()
|
|
||||||
import_root = Path(settings.import_scan_path)
|
import_root = Path(settings.import_scan_path)
|
||||||
|
|
||||||
batch = ImportBatch(
|
batch = ImportBatch(
|
||||||
|
|||||||
Reference in New Issue
Block a user