fix(tradein/estimator): IMV-путь берёт прокси из пула, пустой пул не ломает /estimate (#3386)
All checks were successful
CI Trade-In / browser-tests (pull_request) Has been skipped
CI Trade-In / frontend-checks (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 / changes (pull_request) Successful in 9s
CI / changes (pull_request) Successful in 10s
CI Trade-In / backend-tests (pull_request) Successful in 4m55s
All checks were successful
CI Trade-In / browser-tests (pull_request) Has been skipped
CI Trade-In / frontend-checks (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 / changes (pull_request) Successful in 9s
CI / changes (pull_request) Successful in 10s
CI Trade-In / backend-tests (pull_request) Successful in 4m55s
Оба вызова `evaluate_via_imv` в `_get_or_fetch_imv_cached` шли без `proxy_provider`, а `providers/_proxy.py::curl_proxy_url` считает `use_pool = флаг AND provider is not None` — пул был выключен по построению, curl-сессия уходила на env-прокси SCRAPER_PROXY_URL (мёртвый узел, #2613). Провайдер берётся из уже существующего module-level импорта `app.services.scraper_adapters` (в estimator цикла нет, в отличие от house_imv_backfill — там lazy import вынужденный). Lease — один на вызов IMV, acquire/release внутри `curl_proxy_url`, release в finally на всех выходах. Пустой пул в проде (`NoProxyAvailableError`, в т.ч. завёрнутый — проверка по цепочке причин `caused_by_no_proxy`) остаётся graceful: `_get_or_fetch_imv_cached` возвращает None, ответ отдаётся без IMV-якоря. Причина в логе теперь честная — «пул прокси пуст», а не «fetch failed» (запрос не уходил вовсе).
This commit is contained in:
parent
aeb1b2b3b9
commit
6f97995140
2 changed files with 266 additions and 2 deletions
|
|
@ -51,6 +51,7 @@ from scraper_kit.providers.yandex.valuation import (
|
|||
YandexValuationResult,
|
||||
YandexValuationScraper,
|
||||
)
|
||||
from scraper_kit.proxy_errors import caused_by_no_proxy
|
||||
from sqlalchemy import text
|
||||
from sqlalchemy.orm import Session
|
||||
|
||||
|
|
@ -79,7 +80,7 @@ from app.services.geocoder import (
|
|||
)
|
||||
from app.services.house_metadata import get_house_metadata
|
||||
from app.services.matching.houses import match_house_readonly, match_or_create_house
|
||||
from app.services.scraper_adapters import RealScraperConfig
|
||||
from app.services.scraper_adapters import RealProxyProvider, RealScraperConfig
|
||||
from app.services.scraper_settings import get_scraper_delay
|
||||
from app.tasks.asking_to_sold_ratio import area_bucket
|
||||
|
||||
|
|
@ -794,6 +795,11 @@ async def _get_or_fetch_imv_cached(
|
|||
has_balcony=has_balcony,
|
||||
has_loggia=has_loggia,
|
||||
config=RealScraperConfig(),
|
||||
# #3386: без provider'а `curl_proxy_url` считает use_pool=False (флаг AND
|
||||
# provider is not None) и уходит на env-прокси SCRAPER_PROXY_URL — мёртвый
|
||||
# узел (#2613). Один lease на вызов: acquire/release внутри curl_proxy_url,
|
||||
# release в finally на всех выходах (исключение/таймаут — тоже).
|
||||
proxy_provider=RealProxyProvider(),
|
||||
)
|
||||
save_imv_evaluation(db, result, estimate_id=estimate_id_for_link)
|
||||
logger.info(
|
||||
|
|
@ -827,6 +833,7 @@ async def _get_or_fetch_imv_cached(
|
|||
has_balcony=has_balcony,
|
||||
has_loggia=has_loggia,
|
||||
config=RealScraperConfig(),
|
||||
proxy_provider=RealProxyProvider(), # #3386, см. первый вызов выше
|
||||
)
|
||||
save_imv_evaluation(db, result, estimate_id=estimate_id_for_link)
|
||||
logger.info(
|
||||
|
|
@ -852,7 +859,14 @@ async def _get_or_fetch_imv_cached(
|
|||
logger.warning("imv: transient error, skipping retry in estimator context: %s", e)
|
||||
return None
|
||||
except Exception as e:
|
||||
logger.warning("imv: fetch failed — estimator продолжает без IMV: %s", e)
|
||||
if caused_by_no_proxy(e):
|
||||
# #3386: «пул прокси пуст» — НАША инфраструктура, не сбой фетча и не
|
||||
# transient-ошибка площадки; HTTP-запрос вообще не уходил. Отдельный текст,
|
||||
# чтобы в логах /estimate это не читалось как «Авито отвалился».
|
||||
# Возврат None — тот же graceful путь: ответ отдаётся без IMV-якоря, не 5xx.
|
||||
logger.warning("imv: пул прокси пуст — estimator продолжает без IMV: %s", e)
|
||||
else:
|
||||
logger.warning("imv: fetch failed — estimator продолжает без IMV: %s", e)
|
||||
return None
|
||||
|
||||
|
||||
|
|
|
|||
250
tradein-mvp/backend/tests/test_3386_estimator_imv_proxy_pool.py
Normal file
250
tradein-mvp/backend/tests/test_3386_estimator_imv_proxy_pool.py
Normal file
|
|
@ -0,0 +1,250 @@
|
|||
"""IMV-путь эстиматора берёт прокси из пула, а пустой пул не роняет /estimate (#3386).
|
||||
|
||||
Корень: `estimator._get_or_fetch_imv_cached` звал kit `evaluate_via_imv(config=...)` БЕЗ
|
||||
`proxy_provider`, а `providers/_proxy.py::curl_proxy_url` считает
|
||||
`use_pool = флаг AND proxy_provider is not None` — то есть пул был выключен по построению
|
||||
и curl-сессия уходила на env-прокси `SCRAPER_PROXY_URL` (мёртвый узел, #2613).
|
||||
|
||||
Три проверки по значению:
|
||||
(а) оба вызова (основной + retry с «очищенным» адресом) передают provider'а;
|
||||
(б) пул пуст + environment=production → `_get_or_fetch_imv_cached` отдаёт None
|
||||
(ответ /estimate без IMV-якоря), НЕ исключение, и причина в логе честная —
|
||||
«пул прокси пуст», а не «fetch failed»;
|
||||
(в) lease освобождён ровно один раз и на успехе, и на ошибке IMV.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import logging
|
||||
import os
|
||||
from typing import Any
|
||||
from unittest.mock import AsyncMock, MagicMock, patch
|
||||
|
||||
import anyio
|
||||
import pytest
|
||||
|
||||
os.environ.setdefault("DATABASE_URL", "postgresql+psycopg://test:test@localhost/test_db")
|
||||
|
||||
from scraper_kit.contracts import ProxyLease
|
||||
from scraper_kit.providers.avito.imv import (
|
||||
IMVAddressNotFoundError,
|
||||
IMVEvaluation,
|
||||
IMVGeo,
|
||||
IMVTransientError,
|
||||
)
|
||||
|
||||
from app.core.config import settings
|
||||
from app.services import estimator
|
||||
from app.services.estimator import _get_or_fetch_imv_cached
|
||||
|
||||
|
||||
def _fake_evaluation() -> IMVEvaluation:
|
||||
return IMVEvaluation(
|
||||
cache_key="c" * 64,
|
||||
address="ЕКБ, ул. Тургенева, 4",
|
||||
rooms=2,
|
||||
area_m2=50.0,
|
||||
floor=3,
|
||||
floor_at_home=9,
|
||||
house_type="panel",
|
||||
renovation_type="cosmetic",
|
||||
has_balcony=False,
|
||||
has_loggia=False,
|
||||
geo=IMVGeo(geo_hash="JWT"),
|
||||
recommended_price=6_000_000,
|
||||
lower_price=5_800_000,
|
||||
higher_price=6_300_000,
|
||||
market_count=100,
|
||||
)
|
||||
|
||||
|
||||
def _db_cache_miss() -> MagicMock:
|
||||
db = MagicMock()
|
||||
db.execute.return_value.mappings.return_value.first.return_value = None
|
||||
return db
|
||||
|
||||
|
||||
async def _call(db: Any, *, address: str) -> Any:
|
||||
return await _get_or_fetch_imv_cached(
|
||||
db,
|
||||
address=address,
|
||||
rooms=2,
|
||||
area_m2=50.0,
|
||||
floor=3,
|
||||
floor_at_home=9,
|
||||
house_type="panel",
|
||||
renovation_type="cosmetic",
|
||||
has_balcony=False,
|
||||
has_loggia=False,
|
||||
)
|
||||
|
||||
|
||||
# ── (а) проводка provider'а в оба вызова ────────────────────────────────────
|
||||
|
||||
|
||||
def test_imv_call_passes_proxy_provider() -> None:
|
||||
"""Основной вызов получает proxy_provider (на main здесь None → пул выключен)."""
|
||||
mock_evaluate = AsyncMock(return_value=_fake_evaluation())
|
||||
|
||||
async def _run() -> None:
|
||||
with (
|
||||
patch.object(estimator, "evaluate_via_imv", new=mock_evaluate),
|
||||
patch.object(estimator, "save_imv_evaluation", return_value=1),
|
||||
):
|
||||
assert await _call(_db_cache_miss(), address="ЕКБ, ул. Тургенева, 4") is not None
|
||||
|
||||
anyio.run(_run)
|
||||
_, kwargs = mock_evaluate.call_args
|
||||
assert kwargs.get("proxy_provider") is not None
|
||||
|
||||
|
||||
def test_imv_cleaned_address_retry_passes_proxy_provider() -> None:
|
||||
"""Retry по «очищенному» адресу — второй call site, тот же контракт."""
|
||||
calls: list[dict[str, Any]] = []
|
||||
|
||||
async def _evaluate(**kwargs: Any) -> IMVEvaluation:
|
||||
calls.append(kwargs)
|
||||
if len(calls) == 1:
|
||||
raise IMVAddressNotFoundError("no point")
|
||||
return _fake_evaluation()
|
||||
|
||||
async def _run() -> None:
|
||||
with (
|
||||
patch.object(estimator, "evaluate_via_imv", new=_evaluate),
|
||||
patch.object(estimator, "save_imv_evaluation", return_value=1),
|
||||
):
|
||||
# Префикс из _NOISE_PREFIX_RE — иначе cleaned == address и retry не будет.
|
||||
assert await _call(_db_cache_miss(), address="Склад, ул. Тургенева, 4") is not None
|
||||
|
||||
anyio.run(_run)
|
||||
assert len(calls) == 2, "retry с очищенным адресом не состоялся — тест ничего не проверил"
|
||||
assert calls[1]["address"] == "ул. Тургенева, 4"
|
||||
assert calls[1].get("proxy_provider") is not None
|
||||
|
||||
|
||||
# ── (б) пустой пул в проде: деградация, а не 5xx ────────────────────────────
|
||||
|
||||
|
||||
class _EmptyPoolProvider:
|
||||
"""acquire → None (все узлы avito забанены/заняты)."""
|
||||
|
||||
def acquire(self, provider: str) -> ProxyLease | None:
|
||||
return None
|
||||
|
||||
def release(self, lease: ProxyLease) -> None: # pragma: no cover — не должно звучать
|
||||
raise AssertionError("release без lease")
|
||||
|
||||
def mark_health(self, lease: ProxyLease, ok: bool, **kw: Any) -> None: # pragma: no cover
|
||||
raise AssertionError("mark_health без lease")
|
||||
|
||||
def mark_banned(self, lease: ProxyLease, *, source: str) -> None: # pragma: no cover
|
||||
raise AssertionError("mark_banned без lease")
|
||||
|
||||
|
||||
def test_empty_pool_in_production_degrades_without_imv(
|
||||
monkeypatch: pytest.MonkeyPatch, caplog: pytest.LogCaptureFixture
|
||||
) -> None:
|
||||
"""Пул пуст + прод → None (ответ без IMV) + честная причина в логе, без исключения."""
|
||||
monkeypatch.setattr(settings, "use_proxy_pool_curl", True)
|
||||
monkeypatch.setattr(settings, "environment", "production")
|
||||
|
||||
# Реальный evaluate_via_imv: NoProxyAvailableError поднимается в curl_proxy_url ДО
|
||||
# любого HTTP-запроса, поэтому сеть здесь не нужна и не трогается.
|
||||
def _no_http(*a: Any, **kw: Any) -> Any: # pragma: no cover — не должно вызваться
|
||||
raise AssertionError("HTTP-запрос при пустом пуле — прокси-гейт не сработал")
|
||||
|
||||
result: Any = "unset"
|
||||
|
||||
async def _run() -> None:
|
||||
nonlocal result
|
||||
with (
|
||||
# create=True: на main символа в estimator нет, и без него тест краснел бы
|
||||
# AttributeError'ом («возможности нет»), а не неверным ЗНАЧЕНИЕМ.
|
||||
patch.object(estimator, "RealProxyProvider", _EmptyPoolProvider, create=True),
|
||||
patch("curl_cffi.requests.AsyncSession", _no_http),
|
||||
):
|
||||
result = await _call(_db_cache_miss(), address="ЕКБ, ул. Тургенева, 4")
|
||||
|
||||
with caplog.at_level(logging.WARNING, logger="app.services.estimator"):
|
||||
anyio.run(_run)
|
||||
|
||||
assert result is None
|
||||
assert "пул прокси пуст" in caplog.text, caplog.text
|
||||
assert "fetch failed" not in caplog.text, "причина подменена на неспецифичную"
|
||||
|
||||
|
||||
# ── (в) lease освобождён ровно один раз на обоих исходах ────────────────────
|
||||
|
||||
|
||||
class _CountingProvider:
|
||||
def __init__(self) -> None:
|
||||
self.acquired = 0
|
||||
self.released: list[int] = []
|
||||
self.health: list[bool] = []
|
||||
|
||||
def acquire(self, provider: str) -> ProxyLease | None:
|
||||
self.acquired += 1
|
||||
return ProxyLease(id=7, url="http://pool-node:3128", kind="datacenter", rotate_url=None)
|
||||
|
||||
def release(self, lease: ProxyLease) -> None:
|
||||
self.released.append(lease.id)
|
||||
|
||||
def mark_health(self, lease: ProxyLease, ok: bool, **kw: Any) -> None:
|
||||
self.health.append(ok)
|
||||
|
||||
def mark_banned(self, lease: ProxyLease, *, source: str) -> None:
|
||||
pass
|
||||
|
||||
|
||||
def _run_with_pool(
|
||||
monkeypatch: pytest.MonkeyPatch, *, evaluate_side_effect: Any
|
||||
) -> tuple[Any, _CountingProvider]:
|
||||
monkeypatch.setattr(settings, "use_proxy_pool_curl", True)
|
||||
monkeypatch.setattr(settings, "environment", "production")
|
||||
provider = _CountingProvider()
|
||||
|
||||
session = MagicMock()
|
||||
session.close = AsyncMock()
|
||||
result: Any = "unset"
|
||||
|
||||
async def _run() -> None:
|
||||
nonlocal result
|
||||
with (
|
||||
patch.object(estimator, "RealProxyProvider", lambda: provider, create=True),
|
||||
patch.object(estimator, "save_imv_evaluation", return_value=1),
|
||||
patch("curl_cffi.requests.AsyncSession", lambda *a, **kw: session),
|
||||
# Транспорт нам не интересен — проверяем жизненный цикл lease вокруг него.
|
||||
patch("scraper_kit.providers.avito.imv._warmup", new=AsyncMock()),
|
||||
patch(
|
||||
"scraper_kit.providers.avito.imv._geocode",
|
||||
new=AsyncMock(return_value=IMVGeo(geo_hash="JWT")),
|
||||
),
|
||||
patch(
|
||||
"scraper_kit.providers.avito.imv._imv_evaluate",
|
||||
new=AsyncMock(**evaluate_side_effect),
|
||||
),
|
||||
):
|
||||
result = await _call(_db_cache_miss(), address="ЕКБ, ул. Тургенева, 4")
|
||||
|
||||
anyio.run(_run)
|
||||
return result, provider
|
||||
|
||||
|
||||
def test_lease_released_once_on_success(monkeypatch: pytest.MonkeyPatch) -> None:
|
||||
result, provider = _run_with_pool(
|
||||
monkeypatch, evaluate_side_effect={"return_value": _fake_evaluation()}
|
||||
)
|
||||
assert result is not None
|
||||
assert provider.acquired == 1
|
||||
assert provider.released == [7]
|
||||
assert provider.health == [True]
|
||||
|
||||
|
||||
def test_lease_released_once_on_imv_error(monkeypatch: pytest.MonkeyPatch) -> None:
|
||||
result, provider = _run_with_pool(
|
||||
monkeypatch, evaluate_side_effect={"side_effect": IMVTransientError("502 от Авито")}
|
||||
)
|
||||
assert result is None # graceful: /estimate отвечает без IMV
|
||||
assert provider.acquired == 1
|
||||
assert provider.released == [7]
|
||||
assert provider.health == [False] # ошибка засчитана узлу
|
||||
Loading…
Add table
Reference in a new issue