fix(ptica): «объекта нет в БД» считается пропуском, а не сбоем (#2464) #2974
2 changed files with 113 additions and 3 deletions
|
|
@ -344,14 +344,24 @@ async def scrape_catalog_object(
|
||||||
session: BrowserSession,
|
session: BrowserSession,
|
||||||
obj_id: int,
|
obj_id: int,
|
||||||
snapshot_date: date,
|
snapshot_date: date,
|
||||||
) -> bool:
|
) -> bool | None:
|
||||||
"""Scrape одного объекта: fetch HTML → extract __NEXT_DATA__ → parse → UPDATE.
|
"""Scrape одного объекта: fetch HTML → extract __NEXT_DATA__ → parse → UPDATE.
|
||||||
|
|
||||||
Использует SAVEPOINT (begin_nested) для изоляции per-row ошибок.
|
Использует SAVEPOINT (begin_nested) для изоляции per-row ошибок.
|
||||||
Логирует результат через logger.info.
|
Логирует результат через logger.info.
|
||||||
|
|
||||||
Returns:
|
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)
|
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,
|
obj_id,
|
||||||
snapshot_date,
|
snapshot_date,
|
||||||
)
|
)
|
||||||
return False
|
return None
|
||||||
|
|
||||||
logger.info(
|
logger.info(
|
||||||
"catalog_object scraped obj_id=%d fields=%d rows_updated=%d",
|
"catalog_object scraped obj_id=%d fields=%d rows_updated=%d",
|
||||||
|
|
@ -498,6 +508,10 @@ async def scrape_catalog_objects(
|
||||||
break
|
break
|
||||||
continue
|
continue
|
||||||
consecutive_waf = 0
|
consecutive_waf = 0
|
||||||
|
if ok is None:
|
||||||
|
# Строки в БД нет — это пропуск, а не сбой (см. контракт выше).
|
||||||
|
stats["skipped"] += 1
|
||||||
|
continue
|
||||||
if ok:
|
if ok:
|
||||||
stats["succeeded"] += 1
|
stats["succeeded"] += 1
|
||||||
# Фиксируем сразу, а не одним commit'ом в конце (#2464). Раньше весь
|
# Фиксируем сразу, а не одним commit'ом в конце (#2464). Раньше весь
|
||||||
|
|
|
||||||
|
|
@ -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
|
||||||
Loading…
Add table
Reference in a new issue