refactor: dedupe scheduler logging/runtime, centralize SystemSetting access, fix rankings N+1
Behavior-preserving cleanup (345 tests pass, ruff clean):
- scheduler: replace 62 inline logger.x(json.dumps({...})) calls with a
_log_event helper, and collapse 11 identical _job_runtime dicts into an
_idle_runtime() factory over _JOB_NAMES.
- settings: add app/services/settings_store.py (get_setting/get_value/get_map/
upsert_setting) and route ~13 hand-rolled SystemSetting queries + two
identical _settings_map helpers through it.
- scoring.get_rankings: collapse the per-ticker N+1 (3-4 queries + a commit each)
into 2 bulk reads + a single conditional commit; drop the redundant re-fetch.
Lazy recompute-on-read is preserved. Adds first tests for get_rankings.
Net ~ -245 lines across the touched modules.
This commit is contained in:
@@ -17,6 +17,7 @@ from app.models.settings import SystemSetting
|
||||
from app.models.ticker import Ticker
|
||||
from app.models.trade_setup import TradeSetup
|
||||
from app.models.user import User
|
||||
from app.services import settings_store
|
||||
|
||||
logger = logging.getLogger(__name__)
|
||||
|
||||
@@ -126,18 +127,7 @@ async def reset_password(db: AsyncSession, user_id: int, new_password: str) -> U
|
||||
|
||||
async def toggle_registration(db: AsyncSession, enabled: bool) -> SystemSetting:
|
||||
"""Enable or disable user registration via SystemSetting."""
|
||||
result = await db.execute(
|
||||
select(SystemSetting).where(SystemSetting.key == "registration_enabled")
|
||||
)
|
||||
setting = result.scalar_one_or_none()
|
||||
value = str(enabled).lower()
|
||||
|
||||
if setting is None:
|
||||
setting = SystemSetting(key="registration_enabled", value=value)
|
||||
db.add(setting)
|
||||
else:
|
||||
setting.value = value
|
||||
|
||||
setting = await settings_store.upsert_setting(db, "registration_enabled", str(enabled).lower())
|
||||
await db.commit()
|
||||
await db.refresh(setting)
|
||||
return setting
|
||||
@@ -155,17 +145,7 @@ async def list_settings(db: AsyncSession) -> list[SystemSetting]:
|
||||
|
||||
async def update_setting(db: AsyncSession, key: str, value: str) -> SystemSetting:
|
||||
"""Create or update a system setting."""
|
||||
result = await db.execute(
|
||||
select(SystemSetting).where(SystemSetting.key == key)
|
||||
)
|
||||
setting = result.scalar_one_or_none()
|
||||
|
||||
if setting is None:
|
||||
setting = SystemSetting(key=key, value=value)
|
||||
db.add(setting)
|
||||
else:
|
||||
setting.value = value
|
||||
|
||||
setting = await settings_store.upsert_setting(db, key, value)
|
||||
await db.commit()
|
||||
await db.refresh(setting)
|
||||
return setting
|
||||
@@ -309,10 +289,7 @@ async def update_recommendation_config(
|
||||
|
||||
|
||||
async def get_ticker_universe_default(db: AsyncSession) -> dict[str, str]:
|
||||
result = await db.execute(
|
||||
select(SystemSetting).where(SystemSetting.key == "ticker_universe_default")
|
||||
)
|
||||
setting = result.scalar_one_or_none()
|
||||
setting = await settings_store.get_setting(db, "ticker_universe_default")
|
||||
universe = setting.value if setting else DEFAULT_TICKER_UNIVERSE
|
||||
if universe not in SUPPORTED_TICKER_UNIVERSES:
|
||||
universe = DEFAULT_TICKER_UNIVERSE
|
||||
@@ -579,11 +556,7 @@ async def list_jobs(db: AsyncSession) -> list[dict]:
|
||||
jobs_out = []
|
||||
for name in sorted(VALID_JOB_NAMES):
|
||||
# Check enabled setting
|
||||
key = f"job_{name}_enabled"
|
||||
result = await db.execute(
|
||||
select(SystemSetting).where(SystemSetting.key == key)
|
||||
)
|
||||
setting = result.scalar_one_or_none()
|
||||
setting = await settings_store.get_setting(db, f"job_{name}_enabled")
|
||||
enabled = setting.value == "true" if setting else True # default enabled
|
||||
|
||||
# Get scheduler job info
|
||||
|
||||
@@ -27,10 +27,10 @@ from app.config import settings
|
||||
from app.models.alert import AlertLog
|
||||
from app.models.ohlcv import OHLCVRecord
|
||||
from app.models.score import CompositeScore
|
||||
from app.models.settings import SystemSetting
|
||||
from app.models.sr_level import SRLevel
|
||||
from app.models.ticker import Ticker
|
||||
from app.models.watchlist import WatchlistEntry
|
||||
from app.services import settings_store
|
||||
from app.services.admin_service import get_activation_config, update_setting
|
||||
from app.services.qualification import best_target_probability, setup_qualifies
|
||||
from app.services.rr_scanner_service import get_trade_setups
|
||||
@@ -72,14 +72,9 @@ def _as_bool(value: str | None, default: bool) -> bool:
|
||||
return value.strip().lower() == "true"
|
||||
|
||||
|
||||
async def _settings_map(db: AsyncSession) -> dict[str, str]:
|
||||
keys = [KEY_ENABLED, KEY_TOKEN, KEY_CHAT_ID, KEY_QUALIFIED, KEY_SR, KEY_SCORE_DROP, KEY_DIGEST]
|
||||
result = await db.execute(select(SystemSetting).where(SystemSetting.key.in_(keys)))
|
||||
return {s.key: s.value for s in result.scalars().all()}
|
||||
|
||||
|
||||
async def _resolve(db: AsyncSession) -> dict:
|
||||
stored = await _settings_map(db)
|
||||
keys = [KEY_ENABLED, KEY_TOKEN, KEY_CHAT_ID, KEY_QUALIFIED, KEY_SR, KEY_SCORE_DROP, KEY_DIGEST]
|
||||
stored = await settings_store.get_map(db, keys)
|
||||
|
||||
db_token = (stored.get(KEY_TOKEN) or "").strip()
|
||||
if db_token:
|
||||
|
||||
@@ -10,8 +10,8 @@ from sqlalchemy.ext.asyncio import AsyncSession
|
||||
from app.config import settings
|
||||
from app.dependencies import JWT_ALGORITHM
|
||||
from app.exceptions import AuthenticationError, AuthorizationError, DuplicateError
|
||||
from app.models.settings import SystemSetting
|
||||
from app.models.user import User
|
||||
from app.services import settings_store
|
||||
|
||||
|
||||
async def register(db: AsyncSession, username: str, password: str) -> User:
|
||||
@@ -21,10 +21,7 @@ async def register(db: AsyncSession, username: str, password: str) -> User:
|
||||
and creates a user with role='user' and has_access=False.
|
||||
"""
|
||||
# Check registration toggle
|
||||
result = await db.execute(
|
||||
select(SystemSetting).where(SystemSetting.key == "registration_enabled")
|
||||
)
|
||||
setting = result.scalar_one_or_none()
|
||||
setting = await settings_store.get_setting(db, "registration_enabled")
|
||||
if setting is not None and setting.value.lower() == "false":
|
||||
raise AuthorizationError("Registration is closed")
|
||||
|
||||
|
||||
@@ -31,8 +31,8 @@ from sqlalchemy import select
|
||||
from sqlalchemy.ext.asyncio import AsyncSession
|
||||
|
||||
from app.config import settings
|
||||
from app.models.settings import SystemSetting
|
||||
from app.models.ticker import Ticker
|
||||
from app.services import settings_store
|
||||
from app.services.admin_service import get_activation_config, update_setting
|
||||
from app.services.indicator_service import _extract_ohlcv, compute_atr
|
||||
from app.services.outcome_service import (
|
||||
@@ -741,8 +741,7 @@ async def run_and_store(
|
||||
|
||||
async def get_backtest_report(db: AsyncSession) -> dict | None:
|
||||
"""Return the last cached backtest report, or None if never run."""
|
||||
result = await db.execute(select(SystemSetting).where(SystemSetting.key == KEY_REPORT))
|
||||
setting = result.scalar_one_or_none()
|
||||
setting = await settings_store.get_setting(db, KEY_REPORT)
|
||||
if setting is None:
|
||||
return None
|
||||
try:
|
||||
|
||||
@@ -18,9 +18,8 @@ from sqlalchemy.ext.asyncio import AsyncSession
|
||||
|
||||
from app.config import settings
|
||||
from app.providers.alpaca import AlpacaOHLCVProvider
|
||||
from app.services import settings_store
|
||||
from app.services.admin_service import update_setting
|
||||
from app.models.settings import SystemSetting
|
||||
from sqlalchemy import select
|
||||
|
||||
logger = logging.getLogger(__name__)
|
||||
|
||||
@@ -105,8 +104,7 @@ async def update_market_regime(db: AsyncSession) -> dict:
|
||||
|
||||
async def get_market_regime(db: AsyncSession) -> dict:
|
||||
"""Return the cached regime (computed by the daily job)."""
|
||||
result = await db.execute(select(SystemSetting).where(SystemSetting.key == KEY_REGIME))
|
||||
setting = result.scalar_one_or_none()
|
||||
setting = await settings_store.get_setting(db, KEY_REGIME)
|
||||
if setting is None:
|
||||
return {"label": "unknown", "benchmark": BENCHMARK, "reason": "not computed yet"}
|
||||
try:
|
||||
|
||||
@@ -10,6 +10,7 @@ from __future__ import annotations
|
||||
|
||||
import json
|
||||
import logging
|
||||
from collections import defaultdict
|
||||
from datetime import datetime, timezone
|
||||
|
||||
from sqlalchemy import select
|
||||
@@ -17,8 +18,8 @@ from sqlalchemy.ext.asyncio import AsyncSession
|
||||
|
||||
from app.exceptions import NotFoundError, ValidationError
|
||||
from app.models.score import CompositeScore, DimensionScore
|
||||
from app.models.settings import SystemSetting
|
||||
from app.models.ticker import Ticker
|
||||
from app.services import settings_store
|
||||
|
||||
logger = logging.getLogger(__name__)
|
||||
|
||||
@@ -50,10 +51,7 @@ async def _get_ticker(db: AsyncSession, symbol: str) -> Ticker:
|
||||
|
||||
async def _get_weights(db: AsyncSession) -> dict[str, float]:
|
||||
"""Load scoring weights from SystemSetting, falling back to defaults."""
|
||||
result = await db.execute(
|
||||
select(SystemSetting).where(SystemSetting.key == SCORING_WEIGHTS_KEY)
|
||||
)
|
||||
setting = result.scalar_one_or_none()
|
||||
setting = await settings_store.get_setting(db, SCORING_WEIGHTS_KEY)
|
||||
if setting is not None:
|
||||
try:
|
||||
return json.loads(setting.value)
|
||||
@@ -64,21 +62,7 @@ async def _get_weights(db: AsyncSession) -> dict[str, float]:
|
||||
|
||||
async def _save_weights(db: AsyncSession, weights: dict[str, float]) -> None:
|
||||
"""Persist scoring weights to SystemSetting."""
|
||||
result = await db.execute(
|
||||
select(SystemSetting).where(SystemSetting.key == SCORING_WEIGHTS_KEY)
|
||||
)
|
||||
setting = result.scalar_one_or_none()
|
||||
now = datetime.now(timezone.utc)
|
||||
if setting is not None:
|
||||
setting.value = json.dumps(weights)
|
||||
setting.updated_at = now
|
||||
else:
|
||||
setting = SystemSetting(
|
||||
key=SCORING_WEIGHTS_KEY,
|
||||
value=json.dumps(weights),
|
||||
updated_at=now,
|
||||
)
|
||||
db.add(setting)
|
||||
await settings_store.upsert_setting(db, SCORING_WEIGHTS_KEY, json.dumps(weights))
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
@@ -875,73 +859,62 @@ async def get_rankings(db: AsyncSession) -> dict:
|
||||
Returns dict suitable for RankingResponse.
|
||||
"""
|
||||
weights = await _get_weights(db)
|
||||
tickers = (await db.execute(select(Ticker).order_by(Ticker.symbol))).scalars().all()
|
||||
|
||||
# Get all tickers
|
||||
result = await db.execute(select(Ticker).order_by(Ticker.symbol))
|
||||
tickers = list(result.scalars().all())
|
||||
|
||||
rankings: list[dict] = []
|
||||
for ticker in tickers:
|
||||
# Get composite score
|
||||
comp_result = await db.execute(
|
||||
select(CompositeScore).where(CompositeScore.ticker_id == ticker.id)
|
||||
async def _load_scores() -> tuple[dict[int, CompositeScore], dict[int, dict[str, DimensionScore]]]:
|
||||
comps = {
|
||||
c.ticker_id: c
|
||||
for c in (await db.execute(select(CompositeScore))).scalars().all()
|
||||
}
|
||||
dims: dict[int, dict[str, DimensionScore]] = defaultdict(dict)
|
||||
rows = await db.execute(
|
||||
select(DimensionScore).order_by(DimensionScore.ticker_id, DimensionScore.id)
|
||||
)
|
||||
comp = comp_result.scalar_one_or_none()
|
||||
for ds in rows.scalars().all():
|
||||
dims[ds.ticker_id][ds.dimension] = ds
|
||||
return comps, dims
|
||||
|
||||
# If no composite or stale, recompute
|
||||
# Two bulk reads instead of ~4 queries per ticker.
|
||||
comps, dims_by_ticker = await _load_scores()
|
||||
|
||||
# Lazily recompute any stale/missing scores (kept fresh by the daily scan;
|
||||
# this self-heals tickers that aged out between scans), committing once.
|
||||
recomputed = False
|
||||
for ticker in tickers:
|
||||
comp = comps.get(ticker.id)
|
||||
if comp is None or comp.is_stale:
|
||||
# Recompute stale dimensions first
|
||||
dim_result = await db.execute(
|
||||
select(DimensionScore).where(
|
||||
DimensionScore.ticker_id == ticker.id
|
||||
)
|
||||
)
|
||||
dim_scores = {ds.dimension: ds for ds in dim_result.scalars().all()}
|
||||
dim_scores = dims_by_ticker.get(ticker.id, {})
|
||||
for dim in DIMENSIONS:
|
||||
ds = dim_scores.get(dim)
|
||||
if ds is None or ds.is_stale:
|
||||
await compute_dimension_score(db, ticker.symbol, dim)
|
||||
|
||||
await compute_composite_score(db, ticker.symbol, weights)
|
||||
recomputed = True
|
||||
|
||||
if recomputed:
|
||||
await db.commit()
|
||||
comps, dims_by_ticker = await _load_scores()
|
||||
|
||||
# Re-fetch
|
||||
comp_result = await db.execute(
|
||||
select(CompositeScore).where(CompositeScore.ticker_id == ticker.id)
|
||||
)
|
||||
comp = comp_result.scalar_one_or_none()
|
||||
if comp is None:
|
||||
continue
|
||||
|
||||
dim_result = await db.execute(
|
||||
select(DimensionScore).where(
|
||||
DimensionScore.ticker_id == ticker.id
|
||||
)
|
||||
)
|
||||
dims = [
|
||||
{
|
||||
"dimension": ds.dimension,
|
||||
"score": ds.score,
|
||||
"is_stale": ds.is_stale,
|
||||
"computed_at": ds.computed_at,
|
||||
}
|
||||
for ds in dim_result.scalars().all()
|
||||
]
|
||||
|
||||
rankings.append({
|
||||
rankings = [
|
||||
{
|
||||
"symbol": ticker.symbol,
|
||||
"composite_score": comp.score,
|
||||
"dimensions": dims,
|
||||
})
|
||||
"dimensions": [
|
||||
{
|
||||
"dimension": ds.dimension,
|
||||
"score": ds.score,
|
||||
"is_stale": ds.is_stale,
|
||||
"computed_at": ds.computed_at,
|
||||
}
|
||||
for ds in dims_by_ticker.get(ticker.id, {}).values()
|
||||
],
|
||||
}
|
||||
for ticker in tickers
|
||||
if (comp := comps.get(ticker.id)) is not None
|
||||
]
|
||||
|
||||
# Sort by composite score descending
|
||||
rankings.sort(key=lambda r: r["composite_score"], reverse=True)
|
||||
|
||||
return {
|
||||
"rankings": rankings,
|
||||
"weights": weights,
|
||||
}
|
||||
return {"rankings": rankings, "weights": weights}
|
||||
|
||||
|
||||
async def update_weights(
|
||||
|
||||
@@ -12,12 +12,11 @@ from __future__ import annotations
|
||||
|
||||
import logging
|
||||
|
||||
from sqlalchemy import select
|
||||
from sqlalchemy.ext.asyncio import AsyncSession
|
||||
|
||||
from app.config import settings
|
||||
from app.exceptions import ProviderError, ValidationError
|
||||
from app.models.settings import SystemSetting
|
||||
from app.services import settings_store
|
||||
from app.services.admin_service import update_setting
|
||||
|
||||
logger = logging.getLogger(__name__)
|
||||
@@ -53,15 +52,6 @@ KEY_API_KEY = "sentiment_api_key"
|
||||
KEY_BASE_URL = "sentiment_base_url"
|
||||
|
||||
|
||||
async def _get_settings_map(db: AsyncSession) -> dict[str, str]:
|
||||
result = await db.execute(
|
||||
select(SystemSetting).where(
|
||||
SystemSetting.key.in_([KEY_PROVIDER, KEY_MODEL, KEY_API_KEY, KEY_BASE_URL])
|
||||
)
|
||||
)
|
||||
return {s.key: s.value for s in result.scalars().all()}
|
||||
|
||||
|
||||
def _env_key_for(provider: str) -> str:
|
||||
if provider == "openai":
|
||||
return settings.openai_api_key or ""
|
||||
@@ -90,7 +80,7 @@ def _base_url_for(provider: str, stored_base_url: str) -> str:
|
||||
|
||||
async def _resolve(db: AsyncSession) -> dict:
|
||||
"""Resolve effective config from DB > env > default."""
|
||||
stored = await _get_settings_map(db)
|
||||
stored = await settings_store.get_map(db, [KEY_PROVIDER, KEY_MODEL, KEY_API_KEY, KEY_BASE_URL])
|
||||
|
||||
provider = (stored.get(KEY_PROVIDER) or "").strip().lower()
|
||||
if provider not in VALID_PROVIDERS:
|
||||
|
||||
@@ -0,0 +1,46 @@
|
||||
"""Single source for SystemSetting reads/writes.
|
||||
|
||||
Services used to hand-roll ``select(SystemSetting).where(key == ...)`` +
|
||||
``scalar_one_or_none`` (plus a near-identical get-or-create upsert) in a dozen
|
||||
places. These helpers centralise that. ``upsert_setting`` never commits — the
|
||||
caller owns the transaction. ``updated_at`` is managed by the model's
|
||||
``onupdate`` hook, so callers don't set it.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
from collections.abc import Iterable
|
||||
|
||||
from sqlalchemy import select
|
||||
from sqlalchemy.ext.asyncio import AsyncSession
|
||||
|
||||
from app.models.settings import SystemSetting
|
||||
|
||||
|
||||
async def get_setting(db: AsyncSession, key: str) -> SystemSetting | None:
|
||||
"""Return the SystemSetting row for ``key``, or None if unset."""
|
||||
result = await db.execute(select(SystemSetting).where(SystemSetting.key == key))
|
||||
return result.scalar_one_or_none()
|
||||
|
||||
|
||||
async def get_value(db: AsyncSession, key: str, default: str | None = None) -> str | None:
|
||||
"""Return the stored value for ``key``, or ``default`` if unset."""
|
||||
setting = await get_setting(db, key)
|
||||
return setting.value if setting is not None else default
|
||||
|
||||
|
||||
async def get_map(db: AsyncSession, keys: Iterable[str]) -> dict[str, str]:
|
||||
"""Return a {key: value} map for the given keys that exist."""
|
||||
result = await db.execute(select(SystemSetting).where(SystemSetting.key.in_(list(keys))))
|
||||
return {s.key: s.value for s in result.scalars().all()}
|
||||
|
||||
|
||||
async def upsert_setting(db: AsyncSession, key: str, value: str) -> SystemSetting:
|
||||
"""Create or update a setting. Does NOT commit; caller controls the transaction."""
|
||||
setting = await get_setting(db, key)
|
||||
if setting is None:
|
||||
setting = SystemSetting(key=key, value=value)
|
||||
db.add(setting)
|
||||
else:
|
||||
setting.value = value
|
||||
return setting
|
||||
@@ -20,8 +20,8 @@ from sqlalchemy.ext.asyncio import AsyncSession
|
||||
|
||||
from app.config import settings
|
||||
from app.exceptions import ProviderError, ValidationError
|
||||
from app.models.settings import SystemSetting
|
||||
from app.models.ticker import Ticker
|
||||
from app.services import settings_store
|
||||
|
||||
logger = logging.getLogger(__name__)
|
||||
|
||||
@@ -268,8 +268,7 @@ async def _fetch_universe_symbols_from_public(universe: str) -> tuple[list[str],
|
||||
|
||||
async def _read_cached_symbols(db: AsyncSession, universe: str) -> list[str]:
|
||||
key = f"ticker_universe_cache_{universe}"
|
||||
result = await db.execute(select(SystemSetting).where(SystemSetting.key == key))
|
||||
setting = result.scalar_one_or_none()
|
||||
setting = await settings_store.get_setting(db, key)
|
||||
if setting is None:
|
||||
return []
|
||||
|
||||
@@ -304,15 +303,7 @@ async def _write_cached_symbols(
|
||||
"updated_at": datetime.now(timezone.utc).isoformat(),
|
||||
}
|
||||
|
||||
result = await db.execute(select(SystemSetting).where(SystemSetting.key == key))
|
||||
setting = result.scalar_one_or_none()
|
||||
value = json.dumps(payload)
|
||||
|
||||
if setting is None:
|
||||
db.add(SystemSetting(key=key, value=value))
|
||||
else:
|
||||
setting.value = value
|
||||
|
||||
await settings_store.upsert_setting(db, key, json.dumps(payload))
|
||||
await db.commit()
|
||||
|
||||
|
||||
|
||||
Reference in New Issue
Block a user