lekss361/gendesign

Fork 0

feat(tradein): global scraper delay setting (applies across all scrapers) #485

Merged

lekss361 merged 2 commits from feat/tradein-global-scraper-delay into main

2026-05-23 15:43:28 +00:00

lekss361 commented

2026-05-23 15:28:08 +00:00

Owner

Summary

Add global request delay setting — single source of control over request_delay_sec для ALL scrapers (avito, cian, n1, yandex, domrf, rosreestr). Companion frontend UI в sibling PR feat/tradein-cian-admin-page.

Files

data/sql/053_scraper_settings.sql (NEW, 19 lines) — scraper_settings table
data/sql/054_scraper_settings_global.sql (NEW, 25 lines) — seed 'global' row + per-source defaults
app/services/scraper_settings.py (+130 lines) — _GLOBAL_KEY, refactored cache, get_scraper_delay() = max(per_source, global)
app/services/scrapers/avito.py (+5 lines) — load delay from DB в __init__
app/services/scrapers/cian.py (+3 lines) — same
app/services/scrapers/n1.py (+13 lines) — same + 3 pre-existing lint fixes
app/api/v1/admin.py (+69 lines) — GET /scraper-settings, PUT /scraper-settings/{source}
tests/test_scraper_settings.py (+196 lines, 10 tests)

Schema

scraper_settings table (already exists from earlier — migration 053 idempotent):

CREATE TABLE IF NOT EXISTS scraper_settings (
    id bigserial PRIMARY KEY,
    source text UNIQUE NOT NULL,
    request_delay_sec numeric(6,2) NOT NULL DEFAULT 5.0,
    description text,
    created_at timestamptz DEFAULT NOW(),
    updated_at timestamptz DEFAULT NOW()
);

Migration 054 seeds:

'global' → 0.0 (no floor by default)
'avito' → 7.0, 'cian' → 5.0, 'n1' → 5.0, 'domrf' → 5.0, 'rosreestr' → 5.0

ON CONFLICT DO NOTHING — preserves user-edited values on re-run.

get_scraper_delay() behavior

def get_scraper_delay(source: str) -> float:
    """Returns max(per_source_setting, global_setting)."""
    per_source = _get_setting_cached(key)
    global_value = _get_setting_cached('global')
    return max(per_source, global_value)

global = 0 → per-source value used (current behavior)
global = 10 → all scrapers wait ≥ 10s even if per-source = 5

API endpoints

Method	Path	Body	Response
GET	`/api/v1/admin/scraper-settings`	—	`{settings: [{source, request_delay_sec, description, updated_at}, ...]}`
PUT	`/api/v1/admin/scraper-settings/{source}`	`{source, request_delay_sec (0..60), description?}`	updated row

PUT calls invalidate_cache(source) для immediate effect (next scraper instance reads fresh value).

Validators

request_delay_sec: ge=0.0, le=60.0 — global=0 valid
source: 1..64 chars

Tests (10 pass)

global > per_source → global wins
per_source > global → per_source wins
global=0 → per_source used
Missing DB row → class default
DB error → graceful fallback
Cache invalidation (single + all)
Global row absent → treated as 0
API endpoint smoke (list)

Scrapers updated

avito.py, cian.py, n1.py теперь mirror yandex pattern:

def __init__(self) -> None:
    super().__init__()
    self.request_delay_sec = get_scraper_delay(self.name)

Yandex scrapers already use this pattern (unchanged).

Push verify

LOCAL == REMOTE = bfe56e44

Companion PR

feat/tradein-cian-admin-page — Cian admin UI с slider/input для global delay (calls these endpoints).

Test plan

10 unit tests pass
Ruff clean (new code)
Post-deploy: PUT global=10 → next avito scrape waits ≥10s (vs default 7s)
Cache invalidation works (immediate effect, not after 60s)

## Summary Add **global request delay** setting — single source of control over `request_delay_sec` для ALL scrapers (avito, cian, n1, yandex, domrf, rosreestr). Companion frontend UI в sibling PR `feat/tradein-cian-admin-page`. ## Files - `data/sql/053_scraper_settings.sql` (NEW, 19 lines) — `scraper_settings` table - `data/sql/054_scraper_settings_global.sql` (NEW, 25 lines) — seed `'global'` row + per-source defaults - `app/services/scraper_settings.py` (+130 lines) — `_GLOBAL_KEY`, refactored cache, `get_scraper_delay() = max(per_source, global)` - `app/services/scrapers/avito.py` (+5 lines) — load delay from DB в `__init__` - `app/services/scrapers/cian.py` (+3 lines) — same - `app/services/scrapers/n1.py` (+13 lines) — same + 3 pre-existing lint fixes - `app/api/v1/admin.py` (+69 lines) — `GET /scraper-settings`, `PUT /scraper-settings/{source}` - `tests/test_scraper_settings.py` (+196 lines, 10 tests) ## Schema `scraper_settings` table (already exists from earlier — migration 053 idempotent): ```sql CREATE TABLE IF NOT EXISTS scraper_settings ( id bigserial PRIMARY KEY, source text UNIQUE NOT NULL, request_delay_sec numeric(6,2) NOT NULL DEFAULT 5.0, description text, created_at timestamptz DEFAULT NOW(), updated_at timestamptz DEFAULT NOW() ); ``` Migration 054 seeds: - `'global'` → `0.0` (no floor by default) - `'avito'` → `7.0`, `'cian'` → `5.0`, `'n1'` → `5.0`, `'domrf'` → `5.0`, `'rosreestr'` → `5.0` `ON CONFLICT DO NOTHING` — preserves user-edited values on re-run. ## get_scraper_delay() behavior ```python def get_scraper_delay(source: str) -> float: """Returns max(per_source_setting, global_setting).""" per_source = _get_setting_cached(key) global_value = _get_setting_cached('global') return max(per_source, global_value) ``` - `global = 0` → per-source value used (current behavior) - `global = 10` → all scrapers wait ≥ 10s even if per-source = 5 ## API endpoints | Method | Path | Body | Response | |---|---|---|---| | GET | `/api/v1/admin/scraper-settings` | — | `{settings: [{source, request_delay_sec, description, updated_at}, ...]}` | | PUT | `/api/v1/admin/scraper-settings/{source}` | `{source, request_delay_sec (0..60), description?}` | updated row | PUT calls `invalidate_cache(source)` для immediate effect (next scraper instance reads fresh value). ## Validators - `request_delay_sec`: `ge=0.0, le=60.0` — global=0 valid - `source`: 1..64 chars ## Tests (10 pass) - global > per_source → global wins - per_source > global → per_source wins - global=0 → per_source used - Missing DB row → class default - DB error → graceful fallback - Cache invalidation (single + all) - Global row absent → treated as 0 - API endpoint smoke (list) ## Scrapers updated avito.py, cian.py, n1.py теперь mirror yandex pattern: ```python def __init__(self) -> None: super().__init__() self.request_delay_sec = get_scraper_delay(self.name) ``` Yandex scrapers already use this pattern (unchanged). ## Push verify LOCAL == REMOTE = `bfe56e44` ## Companion PR `feat/tradein-cian-admin-page` — Cian admin UI с slider/input для global delay (calls these endpoints). ## Test plan - [x] 10 unit tests pass - [x] Ruff clean (new code) - [ ] Post-deploy: PUT global=10 → next avito scrape waits ≥10s (vs default 7s) - [ ] Cache invalidation works (immediate effect, not after 60s)

lekss361 added 1 commit 2026-05-23 15:28:09 +00:00

feat(tradein): global scraper delay setting (applies across all scrapers) bfe56e44d6

- SQL migrations 053 (scraper_settings table) + 054 (seed global + per-source rows)
- scraper_settings.py: get_scraper_delay() returns max(per_source, global);
  in-memory cache TTL=60s; invalidate_cache() for immediate effect on PUT
- avito/cian/n1 scrapers load delay from DB in __init__ (mirrors yandex pattern)
- Admin API: GET /scraper-settings (list all), PUT /scraper-settings/{source}
  with cache invalidation on update; CAST(:d AS numeric) per psycopg v3 rules
- 10 unit tests: global>per_source, per_source>global, global=0, DB error fallback,
  cache invalidation, API endpoint smoke

lekss361 commented

2026-05-23 15:31:47 +00:00

Author

Owner

Deep review — BLOCK (rebase required)

PR #484 was merged 27 seconds after #485 was created. #485 is now stale — mergeable: false confirms the conflict. Genuinely useful additions (global delay floor, avito/cian/n1 wiring) but cannot merge as-is.

Hard conflicts with main (post-#484)

data/sql/053_scraper_settings.sql — incompatible schemas:
- main (#484): source text PRIMARY KEY, numeric(5,2), no id/created_at, Field(ge=1.0)
- this PR: id SERIAL PRIMARY KEY, source text UNIQUE, numeric(6,2) DEFAULT 5.0, created_at, Field(ge=0.0)
- Same filename → git merge conflict. Even if path resolved, CREATE TABLE IF NOT EXISTS silently skips → schema mismatch on prod.
app/services/scraper_settings.py — file already exists in main (101 lines, yandex-only). This PR's status: "added" cannot apply over existing file.
app/api/v1/admin.py — ScraperSetting, ScraperSettingUpdate, list_scraper_settings, update_scraper_setting, invalidate_cache import — all already in main from #484. Duplicate route registration + duplicate import.

Required rebase plan

git fetch forgejo && git rebase forgejo/main
Drop data/sql/053_scraper_settings.sql from PR (#484's version stays canonical)

Rewrite 054_scraper_settings_global.sql as ALTER, not CREATE:

BEGIN;
ALTER TABLE scraper_settings ALTER COLUMN request_delay_sec TYPE numeric(6,2);
-- (id/created_at optional — low value; recommend skip to stay minimal)
INSERT INTO scraper_settings (source, request_delay_sec, description) VALUES
    ('global', 0.0, 'Global floor (max with per-source). 0 = use per-source.'),
    ('avito', 7.0, '...'), ('cian', 5.0, '...'), ('n1', 5.0, '...'),
    ('domrf', 5.0, '...'), ('rosreestr', 5.0, '...')
ON CONFLICT (source) DO NOTHING;
COMMIT;

Merge changes into existing scraper_settings.py (don't replace):
- Add _GLOBAL_KEY = "global"
- Extend _DEFAULT_DELAY_BY_SOURCE with avito/cian/n1/domrf/rosreestr
- Add max(per_source, global) logic to get_scraper_delay()
- Handle key == _GLOBAL_KEY → 0.0 fallback
- Keep existing yandex _KEY_ALIASES
Modify existing endpoints in admin.py (don't add duplicates):
- Change ScraperSettingUpdate.Field(ge=1.0, le=60.0) → ge=0.0 (required for global=0)
- Add description to UPDATE path (currently dropped on UPSERT)
- Remove duplicate invalidate_cache import
Keep all 8 wiring changes (avito.py, cian.py, n1.py + tests) — these are the real value-add

Minor (post-rebase)

DOM.RF + Rosreestr scrapers seeded but NOT wired in Python → seed is currently dead. Either wire them or remove the seed rows
PR body says id bigserial but SQL uses SERIAL (int4) — moot since column dropping anyway
Test test_invalidate_cache_single_source doesn't cover "PUT global → all per-source max() recomputes"; consider adding regression test
Multi-replica TTL caveat: PUT only invalidates local cache, peer replicas stale up to 60s. Document in vault when feature stabilizes
No vault entry — create code/modules/tradein/Live_Scraper_Config.md after merge

What's preserved from this PR's value

Extending global delay coverage to avito/cian/n1 (and seed for domrf/rosreestr)
max(per_source, global) floor semantics
Field(ge=0.0) to allow global=0
10 unit tests for max() / cache / fallback / DB-error paths

Not merging. Please rebase and re-request review.

## Deep review — BLOCK (rebase required) PR #484 was merged 27 seconds after #485 was created. #485 is now stale — `mergeable: false` confirms the conflict. Genuinely useful additions (global delay floor, avito/cian/n1 wiring) but cannot merge as-is. ### Hard conflicts with main (post-#484) 1. **`data/sql/053_scraper_settings.sql`** — incompatible schemas: - main (#484): `source text PRIMARY KEY`, `numeric(5,2)`, no `id`/`created_at`, `Field(ge=1.0)` - this PR: `id SERIAL PRIMARY KEY`, `source text UNIQUE`, `numeric(6,2) DEFAULT 5.0`, `created_at`, `Field(ge=0.0)` - Same filename → git merge conflict. Even if path resolved, `CREATE TABLE IF NOT EXISTS` silently skips → schema mismatch on prod. 2. **`app/services/scraper_settings.py`** — file already exists in main (101 lines, yandex-only). This PR's `status: "added"` cannot apply over existing file. 3. **`app/api/v1/admin.py`** — `ScraperSetting`, `ScraperSettingUpdate`, `list_scraper_settings`, `update_scraper_setting`, `invalidate_cache` import — all already in main from #484. Duplicate route registration + duplicate import. ### Required rebase plan 1. `git fetch forgejo && git rebase forgejo/main` 2. **Drop** `data/sql/053_scraper_settings.sql` from PR (#484's version stays canonical) 3. **Rewrite** `054_scraper_settings_global.sql` as ALTER, not CREATE: ```sql BEGIN; ALTER TABLE scraper_settings ALTER COLUMN request_delay_sec TYPE numeric(6,2); -- (id/created_at optional — low value; recommend skip to stay minimal) INSERT INTO scraper_settings (source, request_delay_sec, description) VALUES ('global', 0.0, 'Global floor (max with per-source). 0 = use per-source.'), ('avito', 7.0, '...'), ('cian', 5.0, '...'), ('n1', 5.0, '...'), ('domrf', 5.0, '...'), ('rosreestr', 5.0, '...') ON CONFLICT (source) DO NOTHING; COMMIT; ``` 4. **Merge** changes into existing `scraper_settings.py` (don't replace): - Add `_GLOBAL_KEY = "global"` - Extend `_DEFAULT_DELAY_BY_SOURCE` with avito/cian/n1/domrf/rosreestr - Add `max(per_source, global)` logic to `get_scraper_delay()` - Handle `key == _GLOBAL_KEY → 0.0` fallback - Keep existing yandex `_KEY_ALIASES` 5. **Modify** existing endpoints in `admin.py` (don't add duplicates): - Change `ScraperSettingUpdate.Field(ge=1.0, le=60.0)` → `ge=0.0` (required for `global=0`) - Add `description` to UPDATE path (currently dropped on UPSERT) - Remove duplicate `invalidate_cache` import 6. Keep all 8 wiring changes (`avito.py`, `cian.py`, `n1.py` + tests) — these are the real value-add ### Minor (post-rebase) - DOM.RF + Rosreestr scrapers seeded but NOT wired in Python → seed is currently dead. Either wire them or remove the seed rows - PR body says `id bigserial` but SQL uses `SERIAL` (int4) — moot since column dropping anyway - Test `test_invalidate_cache_single_source` doesn't cover "PUT global → all per-source max() recomputes"; consider adding regression test - Multi-replica TTL caveat: PUT only invalidates local cache, peer replicas stale up to 60s. Document in vault when feature stabilizes - No vault entry — create `code/modules/tradein/Live_Scraper_Config.md` after merge ### What's preserved from this PR's value - Extending global delay coverage to avito/cian/n1 (and seed for domrf/rosreestr) - `max(per_source, global)` floor semantics - `Field(ge=0.0)` to allow `global=0` - 10 unit tests for max() / cache / fallback / DB-error paths Not merging. Please rebase and re-request review.

lekss361 referenced this pull request

2026-05-23 15:32:12 +00:00

feat(tradein-ui): Cian scraper admin page + global request delay control #486

lekss361 added 1 commit 2026-05-23 15:39:06 +00:00

Merge remote-tracking branch 'forgejo/main' into feat/tradein-global-scraper-delay 814530d187

# Conflicts:
#	tradein-mvp/backend/app/services/scraper_settings.py
#	tradein-mvp/backend/data/sql/053_scraper_settings.sql
#	tradein-mvp/backend/tests/test_scraper_settings.py

lekss361 commented

2026-05-23 15:39:53 +00:00

Author

Owner

Merge commit 814530d1 pushed — resolves conflicts с PR #484 (которая уже добавила scraper_settings infrastructure).

Resolution

File	Strategy
`data/sql/053_scraper_settings.sql`	`--theirs` (kept main's `source TEXT PRIMARY KEY` schema from PR #484)
`app/services/scraper_settings.py`	Manual merge: main's deferred-import `_open_session()` + Yandex alias map + наш `_GLOBAL_KEY`, `_get_setting_cached()`, `max(per_source, global)` semantics
`tests/test_scraper_settings.py`	Manual merge: main's 10 tests (call_count adjusted для global lookup) + наши 9 global-logic tests
`app/api/v1/admin.py`	Auto-merge OK; removed main's duplicate `ScraperSetting`/`ScraperSettingUpdate` models + endpoint block; kept ours (`ScraperSettingPayload` с `ge=0.0` — required для `global=0`)

Key design decisions

_open_session() use main's deferred-import pattern (avoids DATABASE_URL validation at import time — tests run без env)
_DEFAULT_DELAY_BY_SOURCE merged: main's Yandex sub-keys + наши avito/cian/n1/domrf/rosreestr
_KEY_ALIASES use main's full Yandex alias map (ours был empty)
ScraperSettingPayload.ge=0.0 overrides main's ge=1.0 — global=0 разрешён ("no floor")

Verified

Main's PR #484 NOT implement max(per_source, global) — только per-source lookup. Наше enhancement additive.
19/19 tests pass
Ruff clean
LOCAL == REMOTE = 814530d1
Final diff vs main: 7 files / +386/-131 (наши additions поверх main)

Ready for re-review.

Merge commit `814530d1` pushed — resolves conflicts с PR #484 (которая уже добавила scraper_settings infrastructure). ## Resolution | File | Strategy | |---|---| | `data/sql/053_scraper_settings.sql` | `--theirs` (kept main's `source TEXT PRIMARY KEY` schema from PR #484) | | `app/services/scraper_settings.py` | Manual merge: main's deferred-import `_open_session()` + Yandex alias map + наш `_GLOBAL_KEY`, `_get_setting_cached()`, `max(per_source, global)` semantics | | `tests/test_scraper_settings.py` | Manual merge: main's 10 tests (call_count adjusted для global lookup) + наши 9 global-logic tests | | `app/api/v1/admin.py` | Auto-merge OK; removed main's duplicate `ScraperSetting`/`ScraperSettingUpdate` models + endpoint block; kept ours (`ScraperSettingPayload` с `ge=0.0` — required для `global=0`) | ## Key design decisions - `_open_session()` use main's deferred-import pattern (avoids `DATABASE_URL` validation at import time — tests run без env) - `_DEFAULT_DELAY_BY_SOURCE` merged: main's Yandex sub-keys + наши avito/cian/n1/domrf/rosreestr - `_KEY_ALIASES` use main's full Yandex alias map (ours был empty) - `ScraperSettingPayload.ge=0.0` overrides main's `ge=1.0` — global=0 разрешён ("no floor") ## Verified - Main's PR #484 NOT implement `max(per_source, global)` — только per-source lookup. Наше enhancement additive. - 19/19 tests pass - Ruff clean - LOCAL == REMOTE = `814530d1` - Final diff vs main: 7 files / +386/-131 (наши additions поверх main) Ready for re-review.

lekss361 merged commit e79c9cec88 into main

2026-05-23 15:43:28 +00:00

lekss361 referenced this pull request from a commit

2026-05-23 15:43:30 +00:00

feat(tradein): global scraper delay setting (applies across all scrapers) (#485)

lekss361 commented

2026-05-23 15:43:54 +00:00

Author

Owner

Merged via deep-code-reviewer — verdict APPROVE.

Rebase plan executed correctly:

053 unchanged (existing #484 table reused)
054 is pure INSERT seed (idempotent via ON CONFLICT DO NOTHING)
scraper_settings.py MODIFIED (not added) — _GLOBAL_KEY + max(per_source, global) layered on top of #484
admin.py MODIFIED — old ScraperSetting* models + endpoints removed, new ScraperSettingPayload + ge=0.0 validator added (no duplication)

Verified:

Schema-compatible with existing 053 (numeric(5,2) fits 0..60 range)
_KEY_ALIASES from #484 preserved → yandex umbrella still works under new max() logic
All 4 yandex sub-scrapers wiring (#484) intact; avito/cian/n1 wiring added by this PR (non-duplicate)
18 tests (10 updated + 8 new) — mock_open.call_count correctly bumped for double DB hits (per-source + global)
psycopg v3: CAST(:d AS numeric) ✅; migration wrapped in BEGIN; COMMIT;
No --no-verify / --force / --amend in branch history

Closes follow-up gap from #486 (Cian admin slider min=0 now accepted by API).

Minor non-blocking follow-ups:

Multi-replica invalidate_cache() only clears receiving process (others wait TTL 60s) — single-replica deploy currently, acceptable
Vault code/modules/tradein/Live_Scraper_Config.md recommended (document max(per_source, global) + 60s TTL + per-process cache caveat)

Post-merge verify steps:

deploy-tradein.yml triggers
Migration 054 applies → 5 per-source rows + 'global'=0 seeded (existing yandex row unchanged)
GET /admin/scraper-settings returns 6 rows (yandex, avito, cian, n1, domrf, rosreestr, global = 7 if all seeded)
PUT /admin/scraper-settings/global body {source: "global", request_delay_sec: 0} → 200 (was rejected before)
PUT global=10 → next avito scrape waits ≥10s

Merged via deep-code-reviewer — verdict APPROVE. Rebase plan executed correctly: - 053 unchanged (existing #484 table reused) - 054 is pure INSERT seed (idempotent via `ON CONFLICT DO NOTHING`) - `scraper_settings.py` MODIFIED (not added) — `_GLOBAL_KEY` + `max(per_source, global)` layered on top of #484 - `admin.py` MODIFIED — old `ScraperSetting*` models + endpoints removed, new `ScraperSettingPayload` + `ge=0.0` validator added (no duplication) Verified: - Schema-compatible with existing 053 (`numeric(5,2)` fits 0..60 range) - `_KEY_ALIASES` from #484 preserved → yandex umbrella still works under new `max()` logic - All 4 yandex sub-scrapers wiring (#484) intact; avito/cian/n1 wiring added by this PR (non-duplicate) - 18 tests (10 updated + 8 new) — `mock_open.call_count` correctly bumped for double DB hits (per-source + global) - psycopg v3: `CAST(:d AS numeric)` ✅; migration wrapped in `BEGIN; COMMIT;` - No `--no-verify` / `--force` / `--amend` in branch history Closes follow-up gap from #486 (Cian admin slider min=0 now accepted by API). Minor non-blocking follow-ups: - Multi-replica `invalidate_cache()` only clears receiving process (others wait TTL 60s) — single-replica deploy currently, acceptable - Vault `code/modules/tradein/Live_Scraper_Config.md` recommended (document `max(per_source, global)` + 60s TTL + per-process cache caveat) Post-merge verify steps: 1. deploy-tradein.yml triggers 2. Migration 054 applies → 5 per-source rows + `'global'=0` seeded (existing yandex row unchanged) 3. `GET /admin/scraper-settings` returns 6 rows (yandex, avito, cian, n1, domrf, rosreestr, global = 7 if all seeded) 4. `PUT /admin/scraper-settings/global` body `{source: "global", request_delay_sec: 0}` → 200 (was rejected before) 5. `PUT global=10` → next avito scrape waits ≥10s

lekss361 referenced this pull request

2026-05-23 20:09:02 +00:00

fix(tradein/avito): anti-bot hardening — sleep + abort + shared session #487