refactor(tradein/scraper-kit): domclick providers on shared _base.py (#2364) #2372

Merged

lekss361 merged 1 commit from feat/tradein-scraper-kit-f4d-domclick into main

2026-07-04 06:24:07 +00:00

lekss361 commented

2026-07-04 06:21:51 +00:00

Owner

What

Migrates providers/domclick/serp.py to build its browser fetcher via the shared build_browser_fetcher(self._config, "domclick") helper instead of constructing BrowserFetcher(source=..., endpoint=...) directly. detail.py needed no change — fetch_detail receives an already-built fetcher from the orchestrator.

Part of scraper_kit migration epic #2277, Group F.

Caveat (confirmed inert)

use_pool now reads config.use_proxy_pool_browser instead of a hardcoded False. Confirmed behaviorally inert: browser_fetcher.py's _pool_proxy AND-gates use_pool with proxy_provider is not None, and this call site never passes proxy_provider (DomClickScraper has no such field) — so the pool path is never taken either way, old or new.

Review

Independent code-reviewer verdict: approve, no blocking or minor issues.

Diff verified minimal/scoped exactly as claimed (kwarg-by-kwarg check: endpoint/source/timeout/proxy_provider resolution identical old vs new).
fetch_detail caller/signature check confirmed no changes needed there.
domclick + cross-provider test files: 99 passed (91+8).
Full backend suite: 3278 passed, 6 skipped, 1 pre-existing flake (test_search_api.py::test_search_cache_hit, 401-vs-200 auth assertion, matches known issue #2208) — no new regressions.
ruff check and ruff format --check clean on the changed file.

Closes #2364.

## What Migrates `providers/domclick/serp.py` to build its browser fetcher via the shared `build_browser_fetcher(self._config, "domclick")` helper instead of constructing `BrowserFetcher(source=..., endpoint=...)` directly. `detail.py` needed no change — `fetch_detail` receives an already-built fetcher from the orchestrator. Part of scraper_kit migration epic #2277, Group F. ## Caveat (confirmed inert) `use_pool` now reads `config.use_proxy_pool_browser` instead of a hardcoded `False`. Confirmed behaviorally inert: `browser_fetcher.py`'s `_pool_proxy` AND-gates `use_pool` with `proxy_provider is not None`, and this call site never passes `proxy_provider` (`DomClickScraper` has no such field) — so the pool path is never taken either way, old or new. ## Review Independent code-reviewer verdict: **approve**, no blocking or minor issues. - Diff verified minimal/scoped exactly as claimed (kwarg-by-kwarg check: endpoint/source/timeout/proxy_provider resolution identical old vs new). - `fetch_detail` caller/signature check confirmed no changes needed there. - domclick + cross-provider test files: 99 passed (91+8). - Full backend suite: 3278 passed, 6 skipped, 1 pre-existing flake (`test_search_api.py::test_search_cache_hit`, 401-vs-200 auth assertion, matches known issue #2208) — no new regressions. - `ruff check` and `ruff format --check` clean on the changed file. Closes #2364.

lekss361 added 1 commit 2026-07-04 06:21:51 +00:00

refactor(tradein/scraper-kit): domclick providers on shared _base.py (#2364 )

CI Trade-In / changes (pull_request) Successful in 10s

Details

CI / changes (pull_request) Successful in 10s

Details

CI Trade-In / frontend-checks (pull_request) Has been skipped

Details

CI / backend-tests (pull_request) Has been skipped

Details

CI / frontend-tests (pull_request) Has been skipped

Details

CI / openapi-codegen-check (pull_request) Has been skipped

Details

CI Trade-In / backend-tests (pull_request) Successful in 1m42s

Details

debe55b8a0

Group F4d (epic #2277, parent #2352). Migrated the ad-hoc BrowserFetcher
construction in providers/domclick/serp.py::fetch_city to the shared
build_browser_fetcher(config, source) helper from Foundation (#2358).

Scope actually touched:
- serp.py: replaced the local BrowserFetcher(source="domclick", endpoint=...)
  construction with build_browser_fetcher(self._config, "domclick").
- detail.py: NO changes needed. Grepped for BrowserFetcher(/AsyncSession(/
  curl_cffi and found none - fetch_detail() receives an already-constructed
  browser_fetcher from the caller (orchestrator owns the session lifecycle,
  per its own module docstring), so there was nothing to migrate here.

Caveat verified (same class as avito, per issue instructions): the old
call site omitted use_pool, so it implicitly defaulted to False via the
BrowserFetcher class default. build_browser_fetcher(config, source) reads
use_pool=config.use_proxy_pool_browser instead - a real difference in the
value passed in. However this call site does not pass proxy_provider
(stays the default None), and BrowserFetcher._pool_proxy computes
`use_pool and proxy_provider is not None` before ever using the pool - so
with proxy_provider=None the effective behavior is identical regardless
of what use_pool evaluates to (confirmed by reading browser_fetcher.py
directly, matching the build_browser_fetcher docstring's own note that
domclick is one of the providers not yet wired to the proxy pool, #2160
P4). No observable behavior change. Documented inline at the call site.

Tests: domclick-specific suite (91 tests: golden parity, sweep, admin
ingest parity, detail + detail kit parity) all green, plus cross-provider
test_scraper_kit_pipeline_parity2.py (8 tests). Full backend suite:
3278 passed, 6 skipped, 1 pre-existing flake (test_search_cache_hit,
#2208) - no new failures.