From 2b804300d52c8ddff648fd381c8f11d6ec73a594 Mon Sep 17 00:00:00 2001 From: npeter83 Date: Tue, 18 Aug 2026 23:54:32 +0200 Subject: [PATCH] fix(scheduler): address /code-review R3 findings on the maintenance handler (3) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Round-3 review of the S3 config-fold, all in routes/scheduler.py: - stale docstring: "Persisted to app_state" -> "Persisted via sysconfig" (the batch moved stores in B-core-11; the comment didn't follow). - drop the manual min/max check in the PATCH — sysconfig.set_value already validates against the same spec bounds; catch its ValueError -> 400, matching the config PATCH pattern (bounds enforced in one place). - fetch sysconfig.spec(...) once into a local in the GET instead of twice. No behaviour change beyond the 400 message wording (untested path). 368 pass. --- backend/app/routes/scheduler.py | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/backend/app/routes/scheduler.py b/backend/app/routes/scheduler.py index 5bf95e09..a43440ad 100644 --- a/backend/app/routes/scheduler.py +++ b/backend/app/routes/scheduler.py @@ -90,14 +90,12 @@ def set_maintenance_settings( db: Session = Depends(get_db), ) -> dict: """Admin-tune the maintenance job's rolling re-validation batch (videos re-checked per - run). Persisted to app_state; the job reads it each run.""" - s = sysconfig.spec("maintenance_revalidate_batch") - if not (s.min <= body.revalidate_batch <= s.max): - raise HTTPException( - status_code=400, - detail=f"revalidate_batch must be between {s.min} and {s.max}", - ) - sysconfig.set_value(db, "maintenance_revalidate_batch", body.revalidate_batch) + run). Persisted via sysconfig; the job reads it each run.""" + # set_value validates against the spec's min/max (one source of bounds), mirroring the config PATCH. + try: + sysconfig.set_value(db, "maintenance_revalidate_batch", body.revalidate_batch) + except (TypeError, ValueError) as exc: + raise HTTPException(status_code=400, detail=str(exc) or "Invalid value") new = sysconfig.get_int(db, "maintenance_revalidate_batch") audit.record(db, admin.id, AuditAction.MAINTENANCE_TUNE, summary=f"revalidate batch = {new}", after={"revalidate_batch": new}) db.commit() @@ -165,6 +163,7 @@ def get_scheduler( "daily_budget": sysconfig.get_int(db, "quota_daily_budget"), "backfill_reserve": sysconfig.get_int(db, "backfill_quota_reserve"), } + maint = sysconfig.spec("maintenance_revalidate_batch") return { **snap, @@ -174,7 +173,7 @@ def get_scheduler( "maintenance": { "revalidate_batch": sysconfig.get_int(db, "maintenance_revalidate_batch"), "revalidate_batch_default": settings.maintenance_revalidate_batch, - "min": sysconfig.spec("maintenance_revalidate_batch").min, - "max": sysconfig.spec("maintenance_revalidate_batch").max, + "min": maint.min, + "max": maint.max, }, }