feat(tradein): secondary_only — параметр расписания, выброшенное считается (#1781)
All checks were successful
CI Trade-In / changes (pull_request) Successful in 8s
CI / changes (pull_request) Successful in 9s
CI Trade-In / frontend-checks (pull_request) Has been skipped
CI Trade-In / browser-tests (pull_request) Has been skipped
CI / backend-tests (pull_request) Has been skipped
CI / frontend-tests (pull_request) Has been skipped
CI / openapi-codegen-check (pull_request) Has been skipped
CI Trade-In / backend-tests (pull_request) Successful in 4m27s
All checks were successful
CI Trade-In / changes (pull_request) Successful in 8s
CI / changes (pull_request) Successful in 9s
CI Trade-In / frontend-checks (pull_request) Has been skipped
CI Trade-In / browser-tests (pull_request) Has been skipped
CI / backend-tests (pull_request) Has been skipped
CI / frontend-tests (pull_request) Has been skipped
CI / openapi-codegen-check (pull_request) Has been skipped
CI Trade-In / backend-tests (pull_request) Successful in 4m27s
`run_cian_full_load` передавал `secondary_only=True` жёстко, поэтому включить новостройки в полный обход можно было только деплоем. Теперь это параметр с ТЕМ ЖЕ дефолтом `True` — поведение прода не меняется ни на строку, но решение становится правкой одной ячейки `scrape_schedules.default_params`, а не выкаткой кода. Откат — тем же движением. Почему это важно именно здесь. Новостройки НЕ пропускаются при запросе: они скачиваются, разбираются и выбрасываются последним шагом (`cian/serp.py`), потому что SERP-параметр `object_type=1` у Cian ненадёжен (~5 % выдачи) и фильтруют по authoritative `listing_segment` после парсинга. Проба бакета берёт `totalOffers` из Redux-состояния SERP и считает `pages_needed = ceil(totalOffers / offers_per_page)`, а `totalOffers` включает ОБЕ категории — то есть страницы с новостройками уже скачаны, лимит страниц и антибан-бюджет за них уже заплачены. Включение стоит ноль дополнительных запросов. Заодно `dropped_novostroyki` сохраняется в counters прогона. Счётчик логировался (`dropped_nb=`), но не персистился, и ответить «сколько инвентаря выбрасывает полный обход» задним числом было нечем: логи за 17.08 уже ротировались — `docker logs --since 120h` не находит ни строки «cian:» ни в одном контейнере. Тот же довод, по которому рядом заведён `partial_buckets`. Копится в атрибуте инстанса, а не аргументом `on_bucket`: у колбэка есть внешние реализации, менять его сигнатуру ради счётчика нельзя. Сброс на каждый прогон — инстанс переиспользуется. Замер, ради которого это делается (прод 21.08): месячный охват свипа cian/novostroyki — 11.7 % против 100 % у cian/vtorichka и avito/novostroyki; 11 993 активные строки, медианный возраст 81 сутки, 10 585 старше 30 суток. Подробности и оговорки — в #1781 и #2994. Двусторонне: против origin/main пять тестов красные, и краснота везде по значению, а не по отсутствию символа — ни одного KeyError. Сообщения перечисляют фактическое состояние («параметра нет в сигнатуре; параметры: [...]», «поля нет в запросе; поля: [...]»). Контроли зелёные с обеих сторон: дефолт остаётся True (иначе правка тихо включила бы сбор новостроек на проде — это отдельное решение с замером); фильтр при `secondary_only=True` остаётся на месте и по-прежнему зависит от флага. pytest tradein-mvp/backend — 4644 passed, 23 skipped. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
parent
9b72e50d18
commit
b9cfdaa040
4 changed files with 158 additions and 2 deletions
|
|
@ -1288,6 +1288,15 @@ class CianFullLoadRequest(BaseModel):
|
||||||
"cian_full_load прогона для resume. Без этого поля — full walk с нуля."
|
"cian_full_load прогона для resume. Без этого поля — full walk с нуля."
|
||||||
),
|
),
|
||||||
)
|
)
|
||||||
|
secondary_only: bool = Field(
|
||||||
|
default=True,
|
||||||
|
description=(
|
||||||
|
"True (дефолт, поведение без изменений) — новостройки отбрасываются "
|
||||||
|
"после разбора. False — сохраняются вместе со вторичкой. Дополнительных "
|
||||||
|
"HTTP-запросов не стоит: страницы с ними всё равно скачиваются, "
|
||||||
|
"totalOffers бакета считает обе категории (#1781)."
|
||||||
|
),
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
@router.post("/scrape/cian-full-load", response_model=CitySweepStartResponse)
|
@router.post("/scrape/cian-full-load", response_model=CitySweepStartResponse)
|
||||||
|
|
@ -1326,6 +1335,7 @@ async def start_cian_full_load(
|
||||||
enrich_detail=payload.enrich_detail,
|
enrich_detail=payload.enrich_detail,
|
||||||
detail_top_n=payload.detail_top_n,
|
detail_top_n=payload.detail_top_n,
|
||||||
resume_run_id=payload.resume_run_id,
|
resume_run_id=payload.resume_run_id,
|
||||||
|
secondary_only=payload.secondary_only,
|
||||||
)
|
)
|
||||||
except Exception:
|
except Exception:
|
||||||
logger.exception("cian-full-load background task run_id=%d crashed", run_id)
|
logger.exception("cian-full-load background task run_id=%d crashed", run_id)
|
||||||
|
|
|
||||||
123
tradein-mvp/backend/tests/test_1781_secondary_only_param.py
Normal file
123
tradein-mvp/backend/tests/test_1781_secondary_only_param.py
Normal file
|
|
@ -0,0 +1,123 @@
|
||||||
|
"""`secondary_only` — параметр расписания, а не хардкод; выброшенное считается (#1781).
|
||||||
|
|
||||||
|
`run_cian_full_load` передавал `secondary_only=True` жёстко (`pipeline.py:3328`), поэтому
|
||||||
|
включить новостройки в полный обход можно было только деплоем. При этом новостройки
|
||||||
|
НЕ пропускаются при запросе — они скачиваются, разбираются и выбрасываются последним
|
||||||
|
шагом (`cian/serp.py:671`), потому что SERP-параметр `object_type=1` у Cian ненадёжен
|
||||||
|
(~5 % выдачи) и фильтруют по authoritative `listing_segment` уже после парсинга.
|
||||||
|
|
||||||
|
Следствие, проверенное по коду: проба бакета берёт `totalOffers` из Redux-состояния
|
||||||
|
SERP и считает `pages_needed = ceil(totalOffers / offers_per_page)`; `totalOffers`
|
||||||
|
включает обе категории. То есть страницы с новостройками уже скачаны, лимит страниц и
|
||||||
|
антибан-бюджет за них уже заплачены — включение стоит НОЛЬ дополнительных запросов.
|
||||||
|
|
||||||
|
Дефолт НЕ меняется: `True`, поведение прода прежнее. Меняется только то, что решение
|
||||||
|
становится правкой одной ячейки `scrape_schedules.default_params`, а не деплоем.
|
||||||
|
|
||||||
|
Замер прода 21.08.2026, ради которого это и делается: месячный охват свипа
|
||||||
|
cian/novostroyki — 11.7 % против 100 % у cian/vtorichka и avito/novostroyki;
|
||||||
|
11 993 активные строки, медианный возраст 81 сутки.
|
||||||
|
"""
|
||||||
|
|
||||||
|
from __future__ import annotations
|
||||||
|
|
||||||
|
import os
|
||||||
|
|
||||||
|
os.environ.setdefault("DATABASE_URL", "postgresql+psycopg://test:test@localhost:5432/test")
|
||||||
|
|
||||||
|
import inspect
|
||||||
|
|
||||||
|
|
||||||
|
def test_full_load_no_longer_hardcodes_secondary_only() -> None:
|
||||||
|
"""Головной: значение берётся из параметра, а не из литерала.
|
||||||
|
|
||||||
|
На origin/main в теле стоит `secondary_only=True` — включить новостройки
|
||||||
|
можно только правкой кода.
|
||||||
|
"""
|
||||||
|
from scraper_kit.orchestration import pipeline
|
||||||
|
|
||||||
|
src = inspect.getsource(pipeline.run_cian_full_load)
|
||||||
|
assert "secondary_only=secondary_only" in src, "значение не пробрасывается из параметра"
|
||||||
|
assert (
|
||||||
|
"secondary_only=True," not in src
|
||||||
|
), "в теле остался хардкод — расписание на него повлиять не сможет"
|
||||||
|
|
||||||
|
|
||||||
|
def test_default_is_unchanged() -> None:
|
||||||
|
"""Контроль: дефолт остаётся True — поведение прода не меняется этим PR.
|
||||||
|
|
||||||
|
Без этого контроля правка «сделать параметром» могла бы тихо включить сбор
|
||||||
|
новостроек на проде, а это отдельное решение с замером (#1781, #2994).
|
||||||
|
"""
|
||||||
|
from scraper_kit.orchestration.pipeline import run_cian_full_load
|
||||||
|
|
||||||
|
params = inspect.signature(run_cian_full_load).parameters
|
||||||
|
assert "secondary_only" in params, (
|
||||||
|
"параметра нет в сигнатуре — значение задано в теле, расписание на него "
|
||||||
|
f"повлиять не может; параметры: {list(params)}"
|
||||||
|
)
|
||||||
|
default = params["secondary_only"].default
|
||||||
|
assert default is True, f"дефолт изменён на {default!r} — это не входило в правку"
|
||||||
|
|
||||||
|
|
||||||
|
def test_admin_request_exposes_the_flag_with_same_default() -> None:
|
||||||
|
"""Параметр доезжает до расписания: у эндпоинта есть поле с тем же дефолтом."""
|
||||||
|
from app.api.v1.admin import CianFullLoadRequest
|
||||||
|
|
||||||
|
поля = CianFullLoadRequest.model_fields
|
||||||
|
assert "secondary_only" in поля, (
|
||||||
|
"поля нет в запросе — параметр не доедет из scrape_schedules.default_params; "
|
||||||
|
f"поля: {sorted(поля)}"
|
||||||
|
)
|
||||||
|
assert поля["secondary_only"].default is True
|
||||||
|
req = CianFullLoadRequest()
|
||||||
|
assert req.secondary_only is True
|
||||||
|
assert CianFullLoadRequest(secondary_only=False).secondary_only is False
|
||||||
|
|
||||||
|
|
||||||
|
def test_counters_persist_dropped_novostroyki() -> None:
|
||||||
|
"""Выброшенное обязано сохраняться, а не только логироваться.
|
||||||
|
|
||||||
|
Счётчик `dropped_nb` печатался в лог, но в `scrape_runs.counters` не попадал, и
|
||||||
|
ответить «сколько инвентаря выбрасывает полный обход» задним числом было нечем:
|
||||||
|
логи за нужную дату уже ротировались. Тот же довод, по которому рядом заведён
|
||||||
|
`partial_buckets`.
|
||||||
|
"""
|
||||||
|
from scraper_kit.orchestration.pipeline import CianFullLoadCounters
|
||||||
|
|
||||||
|
c = CianFullLoadCounters()
|
||||||
|
assert "dropped_novostroyki" in c.to_dict(), c.to_dict()
|
||||||
|
c.dropped_novostroyki = 7
|
||||||
|
assert c.to_dict()["dropped_novostroyki"] == 7
|
||||||
|
|
||||||
|
|
||||||
|
def test_scraper_resets_the_counter_per_run() -> None:
|
||||||
|
"""Контроль: счётчик сбрасывается на каждый прогон.
|
||||||
|
|
||||||
|
Инстанс скрапера переиспользуется; без сброса второй прогон унаследовал бы
|
||||||
|
число первого и записал бы в counters завышенное значение.
|
||||||
|
"""
|
||||||
|
from scraper_kit.providers.cian.serp import CianScraper
|
||||||
|
|
||||||
|
src = inspect.getsource(CianScraper.fetch_all_secondary)
|
||||||
|
assert "self.last_dropped_nb = 0" in src, "нет сброса в начале прогона"
|
||||||
|
assert "self.last_dropped_nb += " in inspect.getsource(
|
||||||
|
CianScraper._paginate_leaf_bucket
|
||||||
|
), "накопление не там, где считается dropped_nb"
|
||||||
|
assert (
|
||||||
|
getattr(CianScraper, "last_dropped_nb", None) == 0
|
||||||
|
), "нет класс-дефолта: атрибут не прочитается, если прогон упал до первого бакета"
|
||||||
|
|
||||||
|
|
||||||
|
def test_filter_still_drops_when_flag_is_on() -> None:
|
||||||
|
"""Контроль от переусердствования: при secondary_only=True фильтр остаётся.
|
||||||
|
|
||||||
|
Правка делает флаг управляемым, а не отменяет его.
|
||||||
|
"""
|
||||||
|
from scraper_kit.providers.cian.serp import CianScraper
|
||||||
|
|
||||||
|
# Фильтр лежит в _paginate_leaf_bucket — том методе, что собирает страницы
|
||||||
|
# бакета; fetch_all_secondary только раздаёт флаг вниз.
|
||||||
|
src = inspect.getsource(CianScraper._paginate_leaf_bucket)
|
||||||
|
assert 'lot.listing_segment != "novostroyki"' in src, "фильтр пропал"
|
||||||
|
assert "if secondary_only:" in src, "фильтр перестал зависеть от флага"
|
||||||
|
|
@ -3147,6 +3147,14 @@ class CianFullLoadCounters:
|
||||||
# Без этого счётчика «сколько бакетов чекпоинт не покрывает» видно только грепом
|
# Без этого счётчика «сколько бакетов чекпоинт не покрывает» видно только грепом
|
||||||
# логов, которые теряются при редеплое.
|
# логов, которые теряются при редеплое.
|
||||||
partial_buckets: int = 0
|
partial_buckets: int = 0
|
||||||
|
# Новостройки, отброшенные фильтром secondary_only ПОСЛЕ скачивания и разбора
|
||||||
|
# (#1781). Ровно тот же довод, что у partial_buckets выше: счётчик логировался
|
||||||
|
# (`dropped_nb=` в cian/serp.py), но не сохранялся, и ответить «сколько инвентаря
|
||||||
|
# выбрасывает полный обход» задним числом было нечем — логи за нужную дату уже
|
||||||
|
# ротировались. Заодно это цена вопроса при обсуждении secondary_only=False:
|
||||||
|
# страницы с этими лотами УЖЕ скачаны, лимит страниц и антибан-бюджет за них
|
||||||
|
# уже заплачены, выбрасывается только результат разбора.
|
||||||
|
dropped_novostroyki: int = 0
|
||||||
|
|
||||||
def to_dict(self) -> dict[str, int]:
|
def to_dict(self) -> dict[str, int]:
|
||||||
return {f.name: getattr(self, f.name) for f in fields(self)}
|
return {f.name: getattr(self, f.name) for f in fields(self)}
|
||||||
|
|
@ -3167,6 +3175,7 @@ async def run_cian_full_load(
|
||||||
concurrency: int = 5,
|
concurrency: int = 5,
|
||||||
resume_run_id: int | None = None,
|
resume_run_id: int | None = None,
|
||||||
region_code: int = DEFAULT_REGION_CODE,
|
region_code: int = DEFAULT_REGION_CODE,
|
||||||
|
secondary_only: bool = True,
|
||||||
) -> CianFullLoadCounters:
|
) -> CianFullLoadCounters:
|
||||||
"""Exhaustive региональный сбор Cian ЕКБ вторички (БЕЗ anchor'ов).
|
"""Exhaustive региональный сбор Cian ЕКБ вторички (БЕЗ anchor'ов).
|
||||||
|
|
||||||
|
|
@ -3325,18 +3334,21 @@ async def run_cian_full_load(
|
||||||
await scraper.fetch_all_secondary(
|
await scraper.fetch_all_secondary(
|
||||||
price_cap_per_bucket=price_cap_per_bucket,
|
price_cap_per_bucket=price_cap_per_bucket,
|
||||||
concurrency=concurrency,
|
concurrency=concurrency,
|
||||||
secondary_only=True,
|
secondary_only=secondary_only,
|
||||||
on_bucket=_on_bucket,
|
on_bucket=_on_bucket,
|
||||||
on_progress=_on_progress,
|
on_progress=_on_progress,
|
||||||
skip_buckets=skip_set if skip_set else None,
|
skip_buckets=skip_set if skip_set else None,
|
||||||
)
|
)
|
||||||
|
|
||||||
|
counters.dropped_novostroyki = getattr(scraper, "last_dropped_nb", 0)
|
||||||
logger.info(
|
logger.info(
|
||||||
"cian-full-load run_id=%d: fetch done — unique=%d ins=%d upd=%d",
|
"cian-full-load run_id=%d: fetch done — unique=%d ins=%d upd=%d "
|
||||||
|
"dropped_novostroyki=%d",
|
||||||
run_id,
|
run_id,
|
||||||
counters.unique_fetched,
|
counters.unique_fetched,
|
||||||
counters.saved_inserted,
|
counters.saved_inserted,
|
||||||
counters.saved_updated,
|
counters.saved_updated,
|
||||||
|
counters.dropped_novostroyki,
|
||||||
)
|
)
|
||||||
runs.update_heartbeat(db, run_id, counters.to_dict())
|
runs.update_heartbeat(db, run_id, counters.to_dict())
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -122,6 +122,10 @@ class CianScraper(BaseScraper):
|
||||||
base_url = "https://ekb.cian.ru"
|
base_url = "https://ekb.cian.ru"
|
||||||
# Класс-дефолт; реальное значение загружается из scraper_settings при создании экземпляра.
|
# Класс-дефолт; реальное значение загружается из scraper_settings при создании экземпляра.
|
||||||
request_delay_sec = 5.0 # консервативно: Cian менее агрессивен чем Avito, но 5s безопасно
|
request_delay_sec = 5.0 # консервативно: Cian менее агрессивен чем Avito, но 5s безопасно
|
||||||
|
# Сколько новостроек отброшено фильтром secondary_only за последний прогон
|
||||||
|
# fetch_all_secondary (#1781). Класс-дефолт нужен, чтобы атрибут читался даже
|
||||||
|
# если прогон упал до первого бакета.
|
||||||
|
last_dropped_nb: int = 0
|
||||||
|
|
||||||
def __init__(
|
def __init__(
|
||||||
self,
|
self,
|
||||||
|
|
@ -401,6 +405,9 @@ class CianScraper(BaseScraper):
|
||||||
"""
|
"""
|
||||||
_buckets = rooms_buckets if rooms_buckets is not None else _DEFAULT_ROOMS_BUCKETS
|
_buckets = rooms_buckets if rooms_buckets is not None else _DEFAULT_ROOMS_BUCKETS
|
||||||
seen: dict[str, ScrapedLot] = {}
|
seen: dict[str, ScrapedLot] = {}
|
||||||
|
# Сброс на КАЖДЫЙ прогон (#1781): инстанс скрапера переиспользуется, и без
|
||||||
|
# сброса счётчик копился бы между вызовами и врал бы в counters второго.
|
||||||
|
self.last_dropped_nb = 0
|
||||||
|
|
||||||
for rooms in _buckets:
|
for rooms in _buckets:
|
||||||
room_label = f"room{'_'.join(str(r) for r in rooms)}"
|
room_label = f"room{'_'.join(str(r) for r in rooms)}"
|
||||||
|
|
@ -671,6 +678,10 @@ class CianScraper(BaseScraper):
|
||||||
filtered = [lot for lot in bucket_lots if lot.listing_segment != "novostroyki"]
|
filtered = [lot for lot in bucket_lots if lot.listing_segment != "novostroyki"]
|
||||||
dropped_nb = collected_this_bucket - len(filtered)
|
dropped_nb = collected_this_bucket - len(filtered)
|
||||||
bucket_lots = filtered
|
bucket_lots = filtered
|
||||||
|
# Копим по всему прогону в атрибут инстанса (#1781): вызывающий кладёт это
|
||||||
|
# в counters прогона. Атрибут, а не аргумент on_bucket — у колбэка уже есть
|
||||||
|
# внешние реализации, менять его сигнатуру ради счётчика нельзя.
|
||||||
|
self.last_dropped_nb += dropped_nb
|
||||||
|
|
||||||
# Дедуп в общий seen
|
# Дедуп в общий seen
|
||||||
for lot in bucket_lots:
|
for lot in bucket_lots:
|
||||||
|
|
|
||||||
Loading…
Add table
Reference in a new issue