fix(ptica): «объекта нет в БД» считается пропуском, а не сбоем (#2464)
All checks were successful
CI Trade-In / changes (pull_request) Successful in 8s
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 / changes (pull_request) Successful in 9s
CI / frontend-tests (pull_request) Has been skipped
CI / openapi-codegen-check (pull_request) Successful in 2m5s
CI / backend-tests (pull_request) Successful in 17m17s

stats["skipped"] был объявлен в контракте, возвращался и печатался в лог — и
никогда не увеличивался. Все исходы сваливались в failed: и WAF-блок, и битый
разбор, и «UPDATE затронул 0 строк». По такому счётчику нельзя отличить временную
помеху от настоящей регрессии разбора, а сам он всегда показывал ноль.

Законный источник пропуска в коде БЫЛ: obj_id берутся из БД, но снимок мог
смениться между выборкой и UPDATE'ом — тогда строки (obj_id, snapshot_date) уже
нет. Ветка с логом «not in DB?» существовала и возвращала False, попадая в failed.

Третье состояние сделано через None, а не новым Literal, намеренно: прежние
True/False сохраняют смысл, поэтому существующие вызывающие и тесты не
переписываются. Проверено прогоном — 531 passed, включая
test_domrf_catalog_object_browsersession_throttle (fake возвращает True) и тесты
предохранителя из #2971, ни один не тронут. Это и был довод против Literal:
менять чужой тест ради своей правки — плохая цена за красоту сигнатуры.

Против origin/main:

  skipped=0, failed=3 вместо skipped=2, failed=1   → падает
  сумма счётчиков сходится с processed  — контроль, зелёный с обеих сторон
  True/False сохраняют смысл            — контроль, зелёный с обеих сторон

Первый контроль ловит «починку», при которой пропуск считался бы дважды или
терялся; второй фиксирует ровно то свойство, ради которого выбран None.

Прогоны: tests/services/scrapers + tests/workers — 531 passed rc=0.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
bot-backend 2026-08-20 16:40:03 +05:00
parent cfa0046b34
commit 80ca7a8a35
2 changed files with 113 additions and 3 deletions

View file

@ -344,14 +344,24 @@ async def scrape_catalog_object(
session: BrowserSession,
obj_id: int,
snapshot_date: date,
) -> bool:
) -> bool | None:
"""Scrape одного объекта: fetch HTML → extract __NEXT_DATA__ → parse → UPDATE.
Использует SAVEPOINT (begin_nested) для изоляции per-row ошибок.
Логирует результат через logger.info.
Returns:
True если UPDATE затронул строку, False при ошибке или 0 rows.
True UPDATE затронул строку;
None ПРОПУСК: строки (obj_id, snapshot_date) в БД нет. Это не сбой:
obj_id берутся из БД, но снимок мог смениться между выборкой и
UPDATE'ом. Раньше этот случай возвращал False и попадал в
счётчик failed вместе с настоящими ошибками, а объявленный в
контракте счётчик skipped всегда оставался нулём (#2464);
False сбой: не скачалось, не распарсилось, упал UPDATE.
Третье состояние сделано через None, а не через новый Literal, намеренно:
прежние True/False сохраняют смысл, поэтому вызывающие и тесты, полагающиеся
на них, не меняются.
"""
logger.info("catalog_object scrape start obj_id=%d snapshot_date=%s", obj_id, snapshot_date)
@ -401,7 +411,7 @@ async def scrape_catalog_object(
obj_id,
snapshot_date,
)
return False
return None
logger.info(
"catalog_object scraped obj_id=%d fields=%d rows_updated=%d",
@ -498,6 +508,10 @@ async def scrape_catalog_objects(
break
continue
consecutive_waf = 0
if ok is None:
# Строки в БД нет — это пропуск, а не сбой (см. контракт выше).
stats["skipped"] += 1
continue
if ok:
stats["succeeded"] += 1
# Фиксируем сразу, а не одним commit'ом в конце (#2464). Раньше весь

View file

@ -0,0 +1,96 @@
"""«Объекта нет в БД» считается пропуском, а не сбоем (#2464).
`stats["skipped"]` был объявлен в контракте, возвращался и печатался в лог и никогда не
увеличивался. Все исходы сваливались в `failed`: и WAF-блок, и битый разбор, и «UPDATE
затронул 0 строк». По такому счётчику нельзя отличить временную помеху от настоящей
регрессии разбора, а сам он всегда показывал ноль.
Законный источник пропуска в коде есть: `obj_id` берутся из БД, но снимок мог смениться
между выборкой и UPDATE'ом — тогда строки `(obj_id, snapshot_date)` уже нет. Ветка с этим
логом («not in DB?») существовала и возвращала False.
Третье состояние сделано через `None`, а не новым `Literal`: прежние True/False сохраняют
смысл, поэтому существующие вызывающие и тесты не переписываются. Это отдельно проверено
контролем ниже.
"""
from __future__ import annotations
import os
os.environ.setdefault("DATABASE_URL", "postgresql+psycopg://test:test@localhost:5432/test")
import asyncio
from datetime import date
from typing import Any
from unittest.mock import MagicMock, patch
_SNAPSHOT = date(2026, 8, 20)
class _FakeSession:
def __init__(self, *_a: Any, **_kw: Any) -> None:
pass
async def __aenter__(self) -> _FakeSession:
return self
async def __aexit__(self, *_exc: Any) -> None:
return None
async def warm_up(self) -> None:
return None
def _run(outcomes: dict[int, Any]) -> dict[str, Any]:
from app.services.scrapers import domrf_catalog_object as mod
async def _fake(_db: Any, _s: Any, obj_id: int, _d: date) -> Any:
return outcomes[obj_id]
with (
patch.object(mod, "BrowserSession", _FakeSession),
patch.object(mod, "scrape_catalog_object", _fake),
):
return asyncio.run(
mod.scrape_catalog_objects(
db=MagicMock(),
obj_ids=list(outcomes),
snapshot_date=_SNAPSHOT,
region_code=66,
)
)
def test_missing_row_counts_as_skipped_not_failed() -> None:
"""Пропуск обязан попасть в skipped, а не в failed.
На origin/main этой ветки нет: skipped остаётся нулём, а всё уходит в failed.
"""
stats = _run({1: True, 2: None, 3: False, 4: None})
assert stats["skipped"] == 2, f"skipped={stats['skipped']}, ожидалось 2: {stats}"
assert stats["failed"] == 1, f"failed={stats['failed']}, ожидалось 1 (только настоящий сбой)"
assert stats["succeeded"] == 1
assert stats["processed"] == 4
def test_counters_sum_to_processed() -> None:
"""Контроль: сумма трёх счётчиков сходится с processed.
Ловит «починку», при которой пропуск считался бы дважды или терялся.
"""
stats = _run({1: True, 2: None, 3: False, 4: True, 5: None, 6: False})
assert stats["succeeded"] + stats["failed"] + stats["skipped"] == stats["processed"]
def test_old_bool_contract_still_works() -> None:
"""Контроль: True/False сохраняют прежний смысл.
Это и есть довод в пользу None вместо нового Literal существующие вызывающие
и тесты (напр. test_domrf_catalog_object_browsersession_throttle) не меняются.
"""
stats = _run({1: True, 2: True, 3: False})
assert stats["succeeded"] == 2
assert stats["failed"] == 1
assert stats["skipped"] == 0