fix(ptica): cancel_geo_job перестаёт подтверждать отмену, которой не было (#2464)
All checks were successful
CI Trade-In / changes (pull_request) Successful in 6s
CI / changes (pull_request) Successful in 9s
CI Trade-In / backend-tests (pull_request) Has been skipped
CI Trade-In / browser-tests (pull_request) Has been skipped
CI Trade-In / frontend-checks (pull_request) Has been skipped
CI / frontend-tests (pull_request) Successful in 1m12s
CI / openapi-codegen-check (pull_request) Successful in 2m5s
CI / backend-tests (pull_request) Successful in 17m13s

Близнец только что исправленного resume: фильтр статуса у cancel был всегда, но
ответ возвращал cancelled=True независимо от того, задел ли UPDATE хоть одну
строку. Несуществующий job_id и уже завершённая задача давали тот же ответ, что
настоящая отмена — оператор и админ-UI получали подтверждение действия, которого
не произошло.

Теперь ответ отражает факт: cancelled=False, текущий статус и причина.

Обоснование обеих правок перенесено из ДОКСТРИНГОВ в комментарии. Причину нашёл
gate openapi-codegen-check: FastAPI кладёт докстринг в OpenAPI-description, оттуда
он попадает в опубликованный контракт и в сгенерированные типы фронта. Внутренние
замеры («все 66 задач в терминальных статусах», номера задач) в публичной схеме не
нужны — это утечка внутренней кухни в контракт, и упавший чек поймал её честно.

После переноса в схеме осталось единственное изменение — однострочное описание
resume_geo_job. Оно верное: старое обещало «paused/failed», новое описывает
фактическое поведение. api-types.ts перегенерирован тем же способом, что в CI
(openapi-typescript + project-local prettier 3.9.0), и после правок pre-commit
регенерация повторена — схема не сдвинулась.

Тесты cancel: 2 красных на origin/main с `assert True is False`; контроль
(законная отмена running) зелёный по обе стороны — новый ключ status он намеренно
не трогает, иначе падал бы там с KeyError, то есть по причине «в ответе нет поля».

pytest tests/api/v1: 362 passed, 1 skipped, rc=0 — прогон на ИТОГОВОМ дереве.
This commit is contained in:
bot-backend 2026-08-20 11:03:27 +05:00
parent af4d2a1853
commit 12a1a6fd2c
3 changed files with 99 additions and 28 deletions

View file

@ -1123,18 +1123,48 @@ def cancel_geo_job(
db: Annotated[Session, Depends(get_db)],
) -> dict[str, Any]:
"""Пометить job как cancelled. Worker увидит при следующей итерации."""
db.execute(
text(
"""
UPDATE nspd_geo_jobs SET status = 'cancelled', finished_at = NOW(),
error = COALESCE(error, 'cancelled by admin')
WHERE job_id = :id AND status IN ('queued','running','paused')
"""
),
{"id": job_id},
# #2464: фильтр статуса здесь был всегда (в отличие от resume ниже), но ответ
# возвращал cancelled=True независимо от того, задел ли UPDATE хоть одну строку.
# Несуществующий job_id и уже завершённая задача давали тот же ответ, что
# настоящая отмена — оператор и админ-UI получали подтверждение действия,
# которого не было.
#
# Обоснование держим в КОММЕНТАРИИ, а не в докстринге: FastAPI кладёт докстринг
# в OpenAPI-description, откуда он попадает в опубликованный контракт и в
# сгенерированные типы фронта (frontend/src/lib/api-types.ts). Внутренние замеры
# там не нужны, а gate openapi-codegen-check честно ловит такое расхождение.
row = (
db.execute(
text(
"""
UPDATE nspd_geo_jobs SET status = 'cancelled', finished_at = NOW(),
error = COALESCE(error, 'cancelled by admin')
WHERE job_id = :id AND status IN ('queued','running','paused')
RETURNING job_id
"""
),
{"id": job_id},
)
.mappings()
.first()
)
if row is None:
current = db.execute(
text("SELECT status FROM nspd_geo_jobs WHERE job_id = :id"),
{"id": job_id},
).scalar()
db.commit()
return {
"job_id": job_id,
"cancelled": False,
"status": current,
"reason": (
"задача не найдена" if current is None else f"статус {current!r} уже терминальный"
),
}
db.commit()
return {"job_id": job_id, "cancelled": True}
return {"job_id": job_id, "cancelled": True, "status": "cancelled"}
@router.post("/geo/jobs/{job_id}/resume")
@ -1142,23 +1172,21 @@ def resume_geo_job(
job_id: int,
db: Annotated[Session, Depends(get_db)],
) -> dict[str, Any]:
"""Re-enqueue задачу из НЕзавершённого состояния (paused / failed / cancelled).
#2464: UPDATE шёл БЕЗ фильтра статуса — в отличие от соседнего cancel_geo_job,
который фильтрует явно. Из-за этого «возобновить» можно было завершённую задачу
(status='done' снова 'queued' и повторный прогон, затирая результат) и уже
бегущую (второй worker на тот же job_id лишние запросы к НСПД, у которого WAF).
Замер на проде 19.08: все 66 задач в терминальных статусах 61 done, 5
cancelled. То есть resume на ЛЮБУЮ существующую делал ровно то, чего не должен.
Второе: ручка возвращала resumed=True всегда, независимо от того, изменилось ли
что-нибудь. Теперь ответ отражает факт: не подошёл статус resumed=False,
текущий статус в ответе, задача НЕ ставится в очередь.
'cancelled' оставлен возобновляемым намеренно: cancel_geo_job ручное действие
оператора, и без этого отменённая по ошибке задача не восстанавливалась бы никак.
"""
"""Re-enqueue задачу из НЕзавершённого состояния (paused / failed / cancelled)."""
# #2464: UPDATE шёл БЕЗ фильтра статуса — в отличие от соседнего cancel_geo_job,
# который фильтрует явно. Из-за этого «возобновить» можно было завершённую задачу
# (done → снова queued и повторный прогон, затирая результат) и уже бегущую
# (второй worker на тот же job_id — лишние запросы к НСПД, у которого WAF).
#
# Замер на проде 19.08: все 66 задач в терминальных статусах — 61 done, 5
# cancelled. То есть resume на ЛЮБУЮ существующую делал ровно то, чего не должен.
#
# Второе: ручка возвращала resumed=True всегда, независимо от того, изменилось ли
# что-нибудь. Теперь ответ отражает факт — статус и причина в ответе, задача НЕ
# ставится в очередь.
#
# 'cancelled' оставлен возобновляемым намеренно: cancel — ручное действие
# оператора, и без этого отменённая по ошибке задача не восстанавливалась бы.
from app.services.job_settings import get_setting_value
from app.workers.tasks.nspd_geo import process_nspd_geo_job

View file

@ -105,3 +105,46 @@ def test_response_reports_the_new_status() -> None:
db = _Db("paused", update_matches=True)
assert _resume(db)["status"] == "queued"
# ── cancel_geo_job: тот же класс — подтверждение действия, которого не было ─────
#
# Фильтр статуса здесь был всегда, но ответ возвращал cancelled=True независимо от
# того, задел ли UPDATE строку. Несуществующий job_id и уже завершённая задача давали
# тот же ответ, что настоящая отмена.
def _cancel(db: _Db) -> dict[str, Any]:
return admin_scrape.cancel_geo_job(job_id=1, db=db) # type: ignore[arg-type]
def test_cancel_of_finished_job_is_not_reported_as_success() -> None:
db = _Db("done", update_matches=False)
out = _cancel(db)
assert out["cancelled"] is False
assert out["status"] == "done"
assert "терминальный" in out["reason"]
def test_cancel_of_missing_job_reports_absence() -> None:
db = _Db(None, update_matches=False)
out = _cancel(db)
assert out["cancelled"] is False
assert out["status"] is None
assert "не найдена" in out["reason"]
def test_cancel_of_running_job_still_works() -> None:
"""Контроль: законная отмена по-прежнему подтверждается.
Проверяется только `cancelled` новый ключ `status` намеренно не трогаем, иначе
контроль падал бы на origin/main с KeyError, то есть «в ответе нет поля», а не
«отмена сломана».
"""
db = _Db("running", update_matches=True)
assert _cancel(db)["cancelled"] is True

View file

@ -1763,7 +1763,7 @@ export interface paths {
put?: never;
/**
* Resume Geo Job
* @description Re-enqueue paused/failed job. Resume idempotent через pending targets.
* @description Re-enqueue задачу из НЕзавершённого состояния (paused / failed / cancelled).
*/
post: operations["resume_geo_job_api_v1_admin_scrape_geo_jobs__job_id__resume_post"];
delete?: never;