feat(config): job intervals live in sysconfig, every setting says when it applies, the unread SponsorBlock default goes
An interval override was a row of its own table beside the one store every other admin knob uses; it is now the `job_interval_<job>` sysconfig key (migration 0110 copies them over and drops the table). The Scheduler page stays its one editor: the Configuration page and the setup wizard neither list nor write those keys, since a write there would skip the reschedule. Each ConfigSpec now states `applies` (required, no default). Every key is `live`, held by a scan that finds no read at import time; a `restart` key is refused until the page can say so. A second scan requires a reader for every key on the Configuration page — `sponsorblock_default` had none since it was added, so it leaves the registry, the translations and the store.
This commit is contained in:
@@ -0,0 +1,56 @@
|
||||
"""Migration 0110's data step, DB-lane: an admin's interval override moves from `scheduler_settings`
|
||||
into sysconfig and back, a row for a job that no longer exists is not carried over, and the unread
|
||||
SponsorBlock default leaves the store.
|
||||
|
||||
The schema half is checked by test_migrations.py; a value lost on the way is invisible there.
|
||||
"""
|
||||
|
||||
from alembic import command
|
||||
from alembic.config import Config
|
||||
from sqlalchemy import text
|
||||
|
||||
from app import scheduler
|
||||
from app.db import engine
|
||||
|
||||
|
||||
def _config(conn) -> dict[str, str]:
|
||||
return dict(conn.execute(text("SELECT key, value FROM system_config")).tuples().all())
|
||||
|
||||
|
||||
def test_an_interval_override_moves_into_sysconfig_and_back(db):
|
||||
cfg = Config("alembic.ini")
|
||||
command.downgrade(cfg, "0109_job_runs")
|
||||
try:
|
||||
with engine.begin() as conn:
|
||||
conn.execute(
|
||||
text(
|
||||
"INSERT INTO scheduler_settings (job_id, interval_minutes)"
|
||||
" VALUES ('rss_poll', 25), ('log_gc', 90), ('retired_job', 5)"
|
||||
)
|
||||
)
|
||||
conn.execute(
|
||||
text(
|
||||
"INSERT INTO system_config (key, value, encrypted) VALUES"
|
||||
" ('sponsorblock_default', 'true', false), ('quota_daily_budget', '5000', false)"
|
||||
)
|
||||
)
|
||||
finally:
|
||||
command.upgrade(cfg, "head")
|
||||
|
||||
with engine.connect() as conn:
|
||||
assert _config(conn) == {
|
||||
"job_interval_rss_poll": "25",
|
||||
"job_interval_log_gc": "90",
|
||||
"quota_daily_budget": "5000",
|
||||
}
|
||||
assert conn.execute(text("SELECT to_regclass('scheduler_settings')")).scalar() is None
|
||||
assert scheduler.load_intervals(db)["rss_poll"] == 25
|
||||
|
||||
command.downgrade(cfg, "0109_job_runs")
|
||||
try:
|
||||
with engine.connect() as conn:
|
||||
back = conn.execute(text("SELECT job_id, interval_minutes FROM scheduler_settings"))
|
||||
assert dict(back.tuples().all()) == {"rss_poll": 25, "log_gc": 90}
|
||||
assert _config(conn) == {"quota_daily_budget": "5000"}
|
||||
finally:
|
||||
command.upgrade(cfg, "head")
|
||||
@@ -12,7 +12,7 @@ from datetime import datetime, timedelta, timezone
|
||||
import pytest
|
||||
from sqlalchemy import select
|
||||
|
||||
from app import jobruns, logs, logsink, quota, scheduler, state
|
||||
from app import jobruns, logs, logsink, quota, scheduler, state, sysconfig
|
||||
from app.models import AuditLog, Channel, JobRun, Notification, QuotaEvent
|
||||
from app.runstatus import RunStatus, RunTrigger
|
||||
|
||||
@@ -411,3 +411,54 @@ def test_startup_closes_what_the_previous_process_left_running(db, monkeypatch):
|
||||
scheduler.shutdown_scheduler()
|
||||
(run,) = _runs(db, "plex_sync")
|
||||
assert run.status == "interrupted"
|
||||
|
||||
|
||||
def test_every_job_has_an_interval_and_every_interval_a_job():
|
||||
assert set(scheduler.JOB_FUNCS) == set(scheduler.JOB_INTERVALS)
|
||||
|
||||
|
||||
def test_a_stored_interval_is_what_the_job_starts_with(db, monkeypatch):
|
||||
sysconfig.set_value(db, "job_interval_rss_poll", 25)
|
||||
db.commit()
|
||||
monkeypatch.setattr(scheduler.settings, "scheduler_enabled", True)
|
||||
scheduler.start_scheduler()
|
||||
try:
|
||||
sched = scheduler._scheduler
|
||||
assert sched is not None
|
||||
assert sched.get_job("rss_poll").trigger.interval == timedelta(minutes=25)
|
||||
default = scheduler.JOB_INTERVALS["enrich"]
|
||||
assert sched.get_job("enrich").trigger.interval == timedelta(minutes=default)
|
||||
finally:
|
||||
scheduler.shutdown_scheduler()
|
||||
|
||||
|
||||
def test_an_interval_set_on_the_scheduler_page_is_stored_and_reschedules_the_job(
|
||||
db, make_user, as_user, live_scheduler
|
||||
):
|
||||
live_scheduler.add_job(lambda: None, "interval", minutes=10, id="backfill")
|
||||
admin = as_user(make_user(role="admin"))
|
||||
r = admin.patch("/api/admin/scheduler/jobs/backfill", json={"interval_minutes": 45})
|
||||
assert r.status_code == 200, r.text
|
||||
assert live_scheduler.get_job("backfill").trigger.interval == timedelta(minutes=45)
|
||||
db.expire_all()
|
||||
assert sysconfig.get_int(db, "job_interval_backfill") == 45
|
||||
assert scheduler.load_intervals(db)["backfill"] == 45
|
||||
# The Configuration page neither lists an interval nor writes one past the reschedule.
|
||||
assert (
|
||||
admin.patch("/api/admin/config/job_interval_backfill", json={"value": 5}).status_code == 404
|
||||
)
|
||||
assert admin.delete("/api/admin/config/job_interval_backfill").status_code == 404
|
||||
assert "scheduler" not in admin.get("/api/admin/config").json()["groups"]
|
||||
db.expire_all()
|
||||
assert sysconfig.get_int(db, "job_interval_backfill") == 45
|
||||
|
||||
|
||||
def test_the_setup_wizard_does_not_write_an_interval(db):
|
||||
from fastapi import HTTPException
|
||||
|
||||
from app.routes.setup import setup_config
|
||||
|
||||
with pytest.raises(HTTPException) as refused:
|
||||
setup_config({"job_interval_rss_poll": 5}, db)
|
||||
assert refused.value.status_code == 400
|
||||
assert setup_config({"quota_daily_budget": 5000}, db)["saved"] == ["quota_daily_budget"]
|
||||
|
||||
@@ -15,6 +15,9 @@ boot, sysconfig holds a stored override to it, and the admin page renders a pick
|
||||
The DB lane (`db` fixture) covers the store; the rest is pure logic.
|
||||
"""
|
||||
|
||||
import ast
|
||||
from pathlib import Path
|
||||
|
||||
import pytest
|
||||
from pydantic import ValidationError
|
||||
|
||||
@@ -37,7 +40,8 @@ def test_each_group_is_one_run_of_keys():
|
||||
def test_every_group_sits_in_exactly_one_section():
|
||||
listed = [g for _, members in sysconfig.SECTIONS for g in members]
|
||||
assert len(listed) == len(set(listed)), listed
|
||||
assert set(listed) == {s.group for s in sysconfig.SPECS}
|
||||
assert set(listed) | set(sysconfig.OWN_PAGE_GROUPS) == {s.group for s in sysconfig.SPECS}
|
||||
assert not set(listed) & set(sysconfig.OWN_PAGE_GROUPS)
|
||||
assert [sid for sid, _ in sysconfig.SECTIONS] == ["access", "youtube", "media", "system"]
|
||||
|
||||
|
||||
@@ -112,3 +116,67 @@ def test_set_value_stores_the_canonical_spelling_and_refuses_anything_else(db):
|
||||
with pytest.raises(ValueError, match="scrape, api"):
|
||||
sysconfig.set_value(db, "search_source", "bing")
|
||||
db.rollback()
|
||||
|
||||
|
||||
# --- when a changed value takes effect --------------------------------------------------------------
|
||||
|
||||
_APP = Path(__file__).resolve().parent.parent / "app"
|
||||
_READERS = {"get", "get_str", "get_int", "get_bool"}
|
||||
|
||||
|
||||
def _key_reads() -> list[tuple[str, str, bool]]:
|
||||
"""Every `sysconfig.get*(db, "<key>")` in the app outside the registry: (key, where, inside a
|
||||
function)."""
|
||||
found: list[tuple[str, str, bool]] = []
|
||||
|
||||
def walk(node: ast.AST, path: Path, in_fn: bool) -> None:
|
||||
for child in ast.iter_child_nodes(node):
|
||||
fn = in_fn or isinstance(child, (ast.FunctionDef, ast.AsyncFunctionDef, ast.Lambda))
|
||||
if (
|
||||
isinstance(child, ast.Call)
|
||||
and isinstance(child.func, ast.Attribute)
|
||||
and isinstance(child.func.value, ast.Name)
|
||||
and child.func.value.id == "sysconfig"
|
||||
and child.func.attr in _READERS
|
||||
and len(child.args) >= 2
|
||||
and isinstance(child.args[1], ast.Constant)
|
||||
and isinstance(child.args[1].value, str)
|
||||
):
|
||||
found.append((child.args[1].value, f"{path.name}:{child.lineno}", in_fn))
|
||||
walk(child, path, fn)
|
||||
|
||||
for path in _APP.rglob("*.py"):
|
||||
if path.name != "sysconfig.py":
|
||||
walk(ast.parse(path.read_text(encoding="utf-8")), path, False)
|
||||
return found
|
||||
|
||||
|
||||
def test_every_key_states_when_it_applies_and_none_needs_a_restart_yet():
|
||||
with pytest.raises(TypeError):
|
||||
sysconfig.ConfigSpec("k", "int", "quota", "quota_daily_budget") # pyright: ignore[reportCallIssue]
|
||||
# The Configuration page cannot tell the admin that a setting waits for a restart; a key that
|
||||
# does needs that first (a badge on it and a banner once it changed).
|
||||
assert {s.applies for s in sysconfig.SPECS} == {"live"}
|
||||
|
||||
|
||||
def test_a_live_key_is_read_where_it_is_used_never_at_import():
|
||||
reads = _key_reads()
|
||||
assert len(reads) > 50, len(
|
||||
reads
|
||||
) # the scan sees the reads (it would pass vacuously otherwise)
|
||||
assert [(k, at) for k, at, in_fn in reads if not in_fn] == []
|
||||
|
||||
|
||||
def test_every_key_on_the_configuration_page_has_a_reader():
|
||||
read = {k for k, _, _ in _key_reads()}
|
||||
page = {s.key for s in sysconfig.SPECS if s.group not in sysconfig.OWN_PAGE_GROUPS}
|
||||
assert sorted(page - read) == []
|
||||
|
||||
|
||||
def test_the_intervals_are_keys_of_the_scheduler_page_only():
|
||||
keys = {s.key for s in sysconfig.SPECS if s.group == "scheduler"}
|
||||
assert keys == {sysconfig.interval_key(j) for j in sysconfig.JOB_INTERVAL_DEFAULTS}
|
||||
assert all(len(k) <= 64 for k in keys) # system_config.key
|
||||
assert sysconfig.config_page_spec("job_interval_rss_poll") is None
|
||||
assert sysconfig.config_page_spec("quota_daily_budget") is not None
|
||||
assert "scheduler" not in sysconfig.describe(_FakeDb())["groups"]
|
||||
|
||||
Reference in New Issue
Block a user