diff --git a/tradein-mvp/backend/app/services/house_imv_backfill.py b/tradein-mvp/backend/app/services/house_imv_backfill.py index a98c93c6..86260d19 100644 --- a/tradein-mvp/backend/app/services/house_imv_backfill.py +++ b/tradein-mvp/backend/app/services/house_imv_backfill.py @@ -25,10 +25,18 @@ from collections.abc import Callable from dataclasses import dataclass, field from typing import Literal +from scraper_kit.browser_fetcher import BrowserFetcher from sqlalchemy import text from sqlalchemy.orm import Session from app.core.config import settings + +# app.services.scrapers.avito_imv остаётся legacy — миграция валюационных +# зависимостей (avito_imv/cian_valuation/yandex_valuation) выделена в отдельный, +# более рискованный issue #2308 (см. epic #2277, issue #2310 Group C). Мигрируем +# здесь только BrowserFetcher (#2310, выше) — avito_imv.evaluate_via_imv принимает +# его исключительно как TYPE_CHECKING-аннотацию (duck-typed на fetch_json), поэтому +# смена конкретного класса не требует правок avito_imv.py. from app.services.scrapers.avito_imv import ( IMVAddressNotFoundError, IMVAuthError, @@ -36,7 +44,6 @@ from app.services.scrapers.avito_imv import ( IMVTransientError, evaluate_via_imv, ) -from app.services.scrapers.browser_fetcher import BrowserFetcher logger = logging.getLogger(__name__) @@ -484,7 +491,7 @@ async def backfill_house_imv( # Флаг OFF → _bf=None → evaluate_via_imv делает свою curl-сессию как раньше # (поведение байт-в-байт идентично доспринтовому). if settings.avito_imv_use_browser_fetcher: - async with BrowserFetcher(source="avito") as _bf: + async with BrowserFetcher(source="avito", endpoint=settings.browser_http_endpoint) as _bf: await _run_loop(_bf) else: await _run_loop(None) diff --git a/tradein-mvp/backend/app/tasks/avito_detail_backfill.py b/tradein-mvp/backend/app/tasks/avito_detail_backfill.py index f736856b..72133d59 100644 --- a/tradein-mvp/backend/app/tasks/avito_detail_backfill.py +++ b/tradein-mvp/backend/app/tasks/avito_detail_backfill.py @@ -23,6 +23,18 @@ from dataclasses import dataclass, field from urllib.parse import urlparse from curl_cffi.requests import AsyncSession +from scraper_kit.avito_exceptions import ( + AvitoBlockedError, + AvitoListingGoneError, + AvitoRateLimitedError, +) +from scraper_kit.browser_fetcher import BrowserFetcher +from scraper_kit.providers.avito.detail import ( + fetch_detail, + research_in_session, + save_detail_enrichment, +) +from scraper_kit.providers.avito.serp import AvitoScraper from sqlalchemy import text from sqlalchemy.orm import Session @@ -30,20 +42,22 @@ from app.core.config import settings from app.core.shutdown import shutdown_requested from app.services import scrape_runs as runs_mod from app.services.scrape_pipeline import _CHROME_HEADERS, _avito_proxies -from app.services.scrapers.avito import AvitoScraper +from app.services.scraper_adapters import RealScraperConfig + +# _AVITO_WARM_SEARCH_URL/build_warmed_session остаются legacy (#2310): kit's +# scraper_kit.providers.avito.detail.build_warmed_session() строит сессию через +# _build_detail_session() БЕЗ config-параметра (сигнатура вообще не принимает +# ScraperConfig — в отличие от fetch_detail, здесь нет caller-side способа +# прокинуть прокси). Мигрировав этот импорт, warm-batch curl-путь (use_curl=True, +# ДЕФОЛТ прод-режима, avito_detail_backfill_use_curl=True) молча перестал бы +# использовать settings.scraper_proxy_url (sticky МГТС-прокси) — прямое +# datacenter-подключение вместо backconnect. Тот же класс бага, что #2322 +# (недостающая инъекция конфига в scraper_kit provider-функции), но другая +# функция — заведён как отдельный follow-up, НЕ #2322. См. отчёт issue #2310. from app.services.scrapers.avito_detail import ( _AVITO_WARM_SEARCH_URL, build_warmed_session, - fetch_detail, - research_in_session, - save_detail_enrichment, ) -from app.services.scrapers.avito_exceptions import ( - AvitoBlockedError, - AvitoListingGoneError, - AvitoRateLimitedError, -) -from app.services.scrapers.browser_fetcher import BrowserFetcher logger = logging.getLogger(__name__) @@ -121,7 +135,10 @@ async def run_avito_detail_backfill( own_session = False own_browser = False - scraper = AvitoScraper() + # kit AvitoScraper требует ScraperConfig позиционно (Strangler-инжекция #2133) — + # RealScraperConfig проксирует settings.* так же, как читал legacy-конструктор без + # аргументов (avito_proxy_rotate_url и т.д. для _rotate_ip()). + scraper = AvitoScraper(RealScraperConfig()) start = time.monotonic() logger.info( @@ -136,7 +153,9 @@ async def run_avito_detail_backfill( # эфемерную _build_detail_session() через settings.scraper_proxy_url (backconnect). # use_curl=False (legacy): browser_mode=True → BrowserFetcher/auv, как раньше. if browser_mode: - browser_fetcher = BrowserFetcher(source="avito") + browser_fetcher = BrowserFetcher( + source="avito", endpoint=settings.browser_http_endpoint + ) await browser_fetcher.__aenter__() own_browser = True scraper._browser = browser_fetcher @@ -314,6 +333,13 @@ async def run_avito_detail_backfill( browser_fetcher=browser_fetcher, referer=_AVITO_WARM_SEARCH_URL if use_curl else None, reconnect_on_block=not use_curl, + # config= обязателен: kit fetch_detail без него читает + # config.scraper_proxy_url=None для backconnect-gate'а + # (elif not use_curl own-session path), теряя reconnect- + # on-403 поведение legacy (settings.scraper_proxy_url + # напрямую). Не влияет на use_curl=True (прод-дефолт) — + # там reconnect_on_block=False уже гасит backconnect. + config=RealScraperConfig(), ), timeout=fetch_timeout_s, ) diff --git a/tradein-mvp/backend/app/tasks/cian_history_backfill.py b/tradein-mvp/backend/app/tasks/cian_history_backfill.py index c6f8c3d8..414afc98 100644 --- a/tradein-mvp/backend/app/tasks/cian_history_backfill.py +++ b/tradein-mvp/backend/app/tasks/cian_history_backfill.py @@ -27,12 +27,17 @@ import logging import time from dataclasses import dataclass, field +from scraper_kit.browser_fetcher import BrowserFetcher +from scraper_kit.providers.cian.detail import fetch_detail, save_detail_enrichment from sqlalchemy import text from sqlalchemy.orm import Session +from app.core.config import settings from app.services.scraper_settings import get_scraper_delay -from app.services.scrapers.browser_fetcher import BrowserFetcher -from app.services.scrapers.cian_detail import fetch_detail, save_detail_enrichment + +# cian_valuation остаётся legacy — валюационные зависимости (avito_imv/ +# cian_valuation/yandex_valuation) выделены в отдельный, более рискованный +# issue #2308 (см. epic #2277, issue #2310 Group C), не в этой миграции. from app.services.scrapers.cian_valuation import estimate_via_cian_valuation logger = logging.getLogger(__name__) @@ -113,7 +118,7 @@ async def backfill_cian_history( else: # One BrowserFetcher instance shared across all listings in this batch. # priceChanges requires JS rendering — curl_cffi returns empty list (#1574). - async with BrowserFetcher(source="cian") as bf: + async with BrowserFetcher(source="cian", endpoint=settings.browser_http_endpoint) as bf: for row in rows: listing_id: int = row["id"] source_url: str = row["source_url"] @@ -157,6 +162,11 @@ async def backfill_cian_history( # ── 2. Houses: missing houses_price_dynamics ────────────────────────────── if do_houses: + # cian_newbuilding остаётся legacy (issue #2310, NOT migrated): kit's + # scraper_kit.providers.cian.newbuilding.fetch_newbuilding() constructs + # BrowserFetcher(source="cian") WITHOUT the now-mandatory endpoint= kwarg + # → TypeError on every call, no caller-side fix possible (issue #2322, + # still open — verified against provider source, not just issue status). from app.services.scrapers.cian_newbuilding import ( fetch_newbuilding, save_newbuilding_enrichment, diff --git a/tradein-mvp/backend/app/tasks/ekb_geoportal_ingest.py b/tradein-mvp/backend/app/tasks/ekb_geoportal_ingest.py index 413e78b9..878745aa 100644 --- a/tradein-mvp/backend/app/tasks/ekb_geoportal_ingest.py +++ b/tradein-mvp/backend/app/tasks/ekb_geoportal_ingest.py @@ -24,14 +24,14 @@ import logging import os import re +from scraper_kit.providers.ekb_geoportal.client import ( + EkbGeoportalClient, + TopoBuilding, +) from sqlalchemy import text from sqlalchemy.orm import Session from app.core.db import SessionLocal -from app.services.scrapers.ekb_geoportal_client import ( - EkbGeoportalClient, - TopoBuilding, -) logger = logging.getLogger(__name__) diff --git a/tradein-mvp/backend/app/tasks/newbuilding_enrich_backfill.py b/tradein-mvp/backend/app/tasks/newbuilding_enrich_backfill.py index e244fc9d..855eb473 100644 --- a/tradein-mvp/backend/app/tasks/newbuilding_enrich_backfill.py +++ b/tradein-mvp/backend/app/tasks/newbuilding_enrich_backfill.py @@ -95,18 +95,18 @@ class NewbuildingEnrichBackfillResult: """Per-run counters for the newbuilding-enrichment backfill.""" # Population sizing (independent of `limit`). - cian_houses_total: int = 0 # houses linked to ext_source='cian_newbuilding' - cian_houses_fetchable: int = 0 # of those, with cian_zhk_url OR resolvable ext_id - cian_houses_pending: int = 0 # fetchable AND not yet fully enriched (force=False) + cian_houses_total: int = 0 # houses linked to ext_source='cian_newbuilding' + cian_houses_fetchable: int = 0 # of those, with cian_zhk_url OR resolvable ext_id + cian_houses_pending: int = 0 # fetchable AND not yet fully enriched (force=False) # Processing (bounded by `limit`). processed: int = 0 skipped_already_enriched: int = 0 succeeded: int = 0 - resolved_zhk_url: int = 0 # ext_id → cian_zhk_url resolved + persisted (#972) - failed_resolve: int = 0 # had only ext_id, resolver returned None - failed_fetch: int = 0 # fetch returned None / raised - failed_save: int = 0 # save raised after a good fetch + resolved_zhk_url: int = 0 # ext_id → cian_zhk_url resolved + persisted (#972) + failed_resolve: int = 0 # had only ext_id, resolver returned None + failed_fetch: int = 0 # fetch returned None / raised + failed_save: int = 0 # save raised after a good fetch # Row-level deltas (how much actually landed). price_dynamics_rows: int = 0 @@ -355,6 +355,16 @@ async def backfill_newbuilding_enrichment( # Import here (not module top) so the heavy curl_cffi scraper stack is only loaded # when the task actually runs — mirrors cian_history_backfill's lazy import and keeps # unit tests able to patch fetch_newbuilding cheaply. + # + # NOT migrated to scraper_kit (issue #2310, Group C): kit's + # scraper_kit.providers.cian.newbuilding.fetch_newbuilding() constructs + # BrowserFetcher(source="cian") WITHOUT the now-mandatory endpoint= kwarg — + # every call raises TypeError, with no caller-side fix possible (the kit + # provider function doesn't expose a config/endpoint hook at all). This is + # issue #2322, verified STILL OPEN by reading the actual provider source + # (not just the issue's open/closed status) at the time of this migration. + # Left on legacy entirely, exactly like Group A (#2305) did for the same + # bug in admin.py's debug endpoints. from app.services.scrapers.cian_newbuilding import ( fetch_newbuilding, resolve_cian_zhk_url_via_search, @@ -371,11 +381,7 @@ async def backfill_newbuilding_enrichment( result.cian_houses_fetchable = sizing["fetchable"] result.cian_houses_pending = sizing["pending"] - rows = ( - db.execute(text(_SELECT_PENDING_HOUSES), {"force": force, "lim": limit}) - .mappings() - .all() - ) + rows = db.execute(text(_SELECT_PENDING_HOUSES), {"force": force, "lim": limit}).mappings().all() logger.info( "newbuilding-enrich backfill: cian_houses total=%d fetchable=%d pending=%d; " @@ -547,9 +553,7 @@ async def backfill_newbuilding_enrichment( db.commit() except Exception as dexc: sp.rollback() - logger.warning( - "reliability dedup failed house_id=%s: %s", house_id, dexc - ) + logger.warning("reliability dedup failed house_id=%s: %s", house_id, dexc) pd_after, rc_after, rv_after = _house_enrichment_counts(db, house_id) result.price_dynamics_rows += max(0, pd_after - pd_before) diff --git a/tradein-mvp/backend/app/tasks/yandex_detail_backfill.py b/tradein-mvp/backend/app/tasks/yandex_detail_backfill.py index 7c8e5968..b6b6f865 100644 --- a/tradein-mvp/backend/app/tasks/yandex_detail_backfill.py +++ b/tradein-mvp/backend/app/tasks/yandex_detail_backfill.py @@ -31,12 +31,12 @@ import time from dataclasses import dataclass, field from curl_cffi.requests import AsyncSession +from scraper_kit.providers.yandex.detail import YandexDetailScraper, save_detail_enrichment from sqlalchemy import text from sqlalchemy.orm import Session from app.core.config import settings from app.services import scrape_runs as runs_mod -from app.services.scrapers.yandex_detail import YandexDetailScraper, save_detail_enrichment logger = logging.getLogger(__name__) diff --git a/tradein-mvp/backend/app/tasks/yandex_newbuilding_sweep.py b/tradein-mvp/backend/app/tasks/yandex_newbuilding_sweep.py index c65ac1d1..8d986540 100644 --- a/tradein-mvp/backend/app/tasks/yandex_newbuilding_sweep.py +++ b/tradein-mvp/backend/app/tasks/yandex_newbuilding_sweep.py @@ -206,6 +206,15 @@ async def enrich_yandex_newbuilding_sweep( Returns: YandexNewbuildingSweepResult со счётчиками. """ + # NOT migrated to scraper_kit (issue #2310, Group C): both + # scraper_kit.providers.yandex.newbuilding.YandexNewbuildingScraper.fetch_jk() + # AND resolve_yandex_jk_slug() construct BrowserFetcher(source="yandex") + # WITHOUT the now-mandatory endpoint= kwarg — every call raises TypeError, + # with no caller-side fix possible (the kit provider functions don't expose + # a config/endpoint hook at all). This is issue #2322, verified STILL OPEN + # by reading the actual provider source (not just the issue's open/closed + # status) at the time of this migration. Left on legacy entirely, exactly + # like Group A (#2305) did for the same bug in admin.py's debug endpoints. from app.services.scrapers.yandex_newbuilding import ( YandexNewbuildingScraper, resolve_yandex_jk_slug, diff --git a/tradein-mvp/backend/tests/tasks/test_avito_detail_backfill.py b/tradein-mvp/backend/tests/tasks/test_avito_detail_backfill.py index 394e570d..5183dd4a 100644 --- a/tradein-mvp/backend/tests/tasks/test_avito_detail_backfill.py +++ b/tradein-mvp/backend/tests/tasks/test_avito_detail_backfill.py @@ -109,6 +109,8 @@ async def test_backfill_empty_snapshot_marks_done() -> None: @pytest.mark.asyncio async def test_backfill_processes_snapshot_to_completion() -> None: """3 listings -> all fetched and enriched, mark_done called.""" + from app.services.scraper_adapters import RealScraperConfig + snapshot = _make_snapshot(3) db = _mock_db(snapshot) runs = MagicMock() @@ -119,7 +121,7 @@ async def test_backfill_processes_snapshot_to_completion() -> None: with ( patch(_SETTINGS, fake_settings), patch(_SESSION, return_value=AsyncMock()), - patch(_SCRAPER), + patch(_SCRAPER) as mock_scraper_cls, patch(_RUNS, runs), patch(_FETCH, mock_fetch), patch(_SAVE, mock_save), @@ -136,6 +138,14 @@ async def test_backfill_processes_snapshot_to_completion() -> None: assert mock_fetch.call_count == 3 runs.mark_done.assert_called_once() runs.mark_failed.assert_not_called() + # #2310 regression guard: kit fetch_detail silently drops the backconnect- + # on-403 retry (and kit AvitoScraper can't read avito_proxy_rotate_url at + # all) unless config=RealScraperConfig() is passed/injected at the call + # site — assert_called()/call_count alone wouldn't catch someone dropping + # that kwarg later (mirrors #2306's test_backfill_wave2.py:282-286 pattern). + _, fetch_call_kwargs = mock_fetch.call_args + assert isinstance(fetch_call_kwargs.get("config"), RealScraperConfig) + assert isinstance(mock_scraper_cls.call_args.args[0], RealScraperConfig) @pytest.mark.asyncio @@ -145,7 +155,7 @@ async def test_backfill_blocked_abort_after_max_consecutive() -> None: #1950 abort-reorder: abort-check ПЕРЕД recovery → на 5-м (аборт-)блоке rotate_ip НЕ дёргается (не тратим recovery на финальном блоке). rotate_ip x4 (блоки 1-4). """ - from app.services.scrapers.avito_exceptions import AvitoBlockedError + from scraper_kit.avito_exceptions import AvitoBlockedError snapshot = _make_snapshot(10) db = _mock_db(snapshot) @@ -267,7 +277,7 @@ async def test_backfill_top_level_exception_marks_failed() -> None: @pytest.mark.asyncio async def test_backfill_rotate_ip_called_on_each_block() -> None: """1 block + 1 success -> rotate_ip called once, enriched=1.""" - from app.services.scrapers.avito_exceptions import AvitoBlockedError + from scraper_kit.avito_exceptions import AvitoBlockedError snapshot = _make_snapshot(2) db = _mock_db(snapshot) @@ -379,6 +389,7 @@ async def test_backfill_fetch_timeout_skips_and_continues() -> None: browser_fetcher: object = None, referer: object = None, reconnect_on_block: bool = True, + config: object = None, ): call_urls.append(url) if len(call_urls) == 1: @@ -423,7 +434,7 @@ async def test_backfill_listing_gone_marks_inactive_no_breaker() -> None: состоит из dead-листингов; раньше 404 ловился как soft-block → breaker абортил run до live-листингов. Теперь 404 нейтрален к breaker'у и метит листинг inactive. """ - from app.services.scrapers.avito_exceptions import AvitoListingGoneError + from scraper_kit.avito_exceptions import AvitoListingGoneError snapshot = _make_snapshot(1) db = _mock_db(snapshot) @@ -541,8 +552,11 @@ async def test_backfill_use_curl_false_creates_browser_fetcher() -> None: db, run_id=10, params={"batch_size": 10, "budget_sec": 3600} ) - # BrowserFetcher должен быть создан (source="avito") - mock_bf_cls.assert_called_once_with(source="avito") + # BrowserFetcher должен быть создан (source="avito", endpoint из settings — #2310 + # kit BrowserFetcher требует endpoint= обязательным keyword-only параметром) + mock_bf_cls.assert_called_once_with( + source="avito", endpoint=fake_settings.browser_http_endpoint + ) # fetch_detail вызван с browser_fetcher установленным (не None) assert mock_fetch.call_count == 1 _, kwargs = mock_fetch.call_args @@ -560,7 +574,7 @@ async def test_backfill_use_curl_block_cooldown_research_no_rebuild() -> None: нельзя. Блок = rate-limit текущего IP → cooldown + re-search той же сессией. fetch: 1-й вызов BLOCKED, 2-й — успех. """ - from app.services.scrapers.avito_exceptions import AvitoBlockedError + from scraper_kit.avito_exceptions import AvitoBlockedError snapshot = _make_snapshot(2) db = _mock_db(snapshot) diff --git a/tradein-mvp/backend/tests/test_scraper_kit_group_c_backfill_kit_parity.py b/tradein-mvp/backend/tests/test_scraper_kit_group_c_backfill_kit_parity.py new file mode 100644 index 00000000..04c067ad --- /dev/null +++ b/tradein-mvp/backend/tests/test_scraper_kit_group_c_backfill_kit_parity.py @@ -0,0 +1,297 @@ +"""Parity tests for issue #2310 (Group C of scraper_kit migration epic #2277). + +Migrates legacy scraper imports of these files to their `scraper_kit` equivalents +(behavior of the enclosing task/service functions is unchanged — see vault audit +`Scraper_Kit_Legacy_Dependency_Audit_0703`): + + - app/services/house_imv_backfill.py → BrowserFetcher now from + `scraper_kit.browser_fetcher` (was `app.services.scrapers.browser_fetcher`). + avito_imv itself stays legacy (valuation dependency, issue #2308 — separate). + - app/tasks/avito_detail_backfill.py → AvitoScraper/BrowserFetcher/avito + exceptions/fetch_detail/research_in_session/save_detail_enrichment now from + `scraper_kit.*`. `build_warmed_session`/`_AVITO_WARM_SEARCH_URL` stay legacy — + kit's `build_warmed_session()` builds its curl_cffi session via + `_build_detail_session()` with NO config parameter at all (unlike + `fetch_detail`, which does), so there is no caller-side way to inject + `settings.scraper_proxy_url` — migrating it would silently drop the sticky + MGTS-proxy egress on the warm-batch path (avito_detail_backfill_use_curl=True, + the PROD DEFAULT). Same bug category as issue #2322 (scraper_kit provider + missing a config hook), different function — flagged as a new follow-up. + - app/tasks/cian_history_backfill.py → fetch_detail/save_detail_enrichment + (listings block) + BrowserFetcher now from `scraper_kit.*`. The houses block + (cian_newbuilding.fetch_newbuilding/save_newbuilding_enrichment) and the + valuations block (cian_valuation.estimate_via_cian_valuation) stay legacy — + issue #2322 (newbuilding BrowserFetcher missing endpoint=) resp. issue #2308 + (valuation dependencies), both out of scope here. + - app/tasks/ekb_geoportal_ingest.py → EkbGeoportalClient/TopoBuilding now + from `scraper_kit.providers.ekb_geoportal.client` (already proven byte- + identical by tests/scrapers/test_ekb_geoportal_client_kit_parity.py, #2307). + +This file proves the DELTA introduced by the #2310 migration specifically — +general avito/cian-provider parsing golden-parity is already covered by +`tests/test_scraper_kit_avito_golden_parity.py` and +`tests/test_scraper_kit_cian_golden_parity.py`. Uses `tests/support/parity.assert_parity` +per the epic's parity-gate convention (issue #2304). +""" + +from __future__ import annotations + +import asyncio +import dataclasses +import json +import os +from typing import Any +from unittest.mock import AsyncMock, MagicMock, patch + +# Legacy app.services.scrapers.* / app.core.config.settings require DATABASE_URL at +# import time (same pattern as tests/scrapers/test_avito_detail_kit_parity.py). +os.environ.setdefault("DATABASE_URL", "postgresql+psycopg://test:test@localhost/test_db") + +from scraper_kit.avito_exceptions import AvitoBlockedError as KitAvitoBlockedError +from scraper_kit.providers.avito.detail import fetch_detail as kit_avito_fetch_detail +from scraper_kit.providers.avito.serp import AvitoScraper as KitAvitoScraper +from scraper_kit.providers.cian.detail import fetch_detail as kit_cian_fetch_detail + +from app.core.config import settings +from app.services.scraper_adapters import RealScraperConfig +from app.services.scrapers.avito_detail import fetch_detail as legacy_avito_fetch_detail +from app.services.scrapers.cian_detail import fetch_detail as legacy_cian_fetch_detail +from tests.support.parity import assert_parity + +# ── avito fetch_detail: backconnect config-gate (avito_detail_backfill.py delta) ── + + +def _mock_session_403_then_ok(html: str) -> MagicMock: + """curl_cffi-like session: first .get() -> 403, second -> 200 with html.""" + resp_403 = MagicMock(status_code=403, text="") + resp_ok = MagicMock(status_code=200, text=html) + session = MagicMock() + session.get = AsyncMock(side_effect=[resp_403, resp_ok]) + session.close = AsyncMock() + return session + + +def _detail_html() -> str: + return ( + '