Merge pull request 'fix(tradein/geocode): бэкфилл listings.city из слага города в URL Авито (#2594)' (#2606) from fix/tradein-backfill-listing-city-from-url into main
All checks were successful
Deploy Trade-In / changes (push) Successful in 13s
Deploy Trade-In / build-frontend (push) Has been skipped
Deploy Trade-In / build-browser (push) Has been skipped
Deploy Trade-In / test (push) Successful in 2m22s
Deploy Trade-In / build-backend (push) Successful in 29s
Deploy Trade-In / deploy (push) Successful in 1m36s
All checks were successful
Deploy Trade-In / changes (push) Successful in 13s
Deploy Trade-In / build-frontend (push) Has been skipped
Deploy Trade-In / build-browser (push) Has been skipped
Deploy Trade-In / test (push) Successful in 2m22s
Deploy Trade-In / build-backend (push) Successful in 29s
Deploy Trade-In / deploy (push) Successful in 1m36s
This commit is contained in:
commit
87693aec9b
2 changed files with 250 additions and 0 deletions
|
|
@ -0,0 +1,92 @@
|
|||
-- 197_backfill_listings_city_from_url.sql
|
||||
-- Issue #2594 шаг 3 — бэкфилл listings.city (миграция 196) для УЖЕ накопленных
|
||||
-- Avito-объявлений из слага города в source_url.
|
||||
--
|
||||
-- ПРОБЛЕМА: 196 добавила колонку listings.city и write-path проставляет её
|
||||
-- ТОЛЬКО для новых листингов (см. заголовок 196). Накопленные ранее строки
|
||||
-- остались с city IS NULL. Для Avito-объявлений вне ЕКБ (city-sweep областных
|
||||
-- городов) адрес в тексте часто без города («пр-т Вагоностроителей,18» вместо
|
||||
-- «Нижний Тагил, пр-т Вагоностроителей,18»), а у части улиц есть тёзки в
|
||||
-- Екатеринбурге (Хохрякова, Калинина — центральные ЕКБ-улицы). Без явного
|
||||
-- city такой адрес при геокодировании (app/tasks/geocode_missing.py,
|
||||
-- app/services/geocoder.py city_hint) считается «город не назван» → рискует
|
||||
-- получить координаты Екатеринбурга (тот же баг-класс, что и #2594 основной).
|
||||
-- Ночной прогон geocode_missing_listings 2026-08-01 заберёт в очередь 148
|
||||
-- активных объявлений Нижнего Тагила без city — этот бэкфилл проставляет им
|
||||
-- city ДО того, как очередь начнёт их обрабатывать.
|
||||
--
|
||||
-- ИСТОЧНИК: первый сегмент пути URL после хоста —
|
||||
-- https://www.avito.ru/nizhniy_tagil/kvartiry/... -> 'nizhniy_tagil'
|
||||
-- извлекается regex `substring(source_url from 'avito\.ru/([^/]+)/')`.
|
||||
-- Маппинг ТОЛЬКО наших шести городов Свердловской обл. (region 66); слаги и
|
||||
-- человекочитаемые названия сверены с CITY_DISPLAY_NAMES/CITY_LOCATIONS
|
||||
-- (tradein-mvp/packages/scraper-kit/src/scraper_kit/orchestration/pipeline.py)
|
||||
-- — значения побайтно совпадают с тем, что теперь пишет скрапер (go-forward
|
||||
-- write-path 196), чтобы не расщепить один город на две разные метки.
|
||||
--
|
||||
-- Проверено на проде (SELECT, read-only) перед миграцией:
|
||||
-- avito_slug наш город city IS NULL (Avito)
|
||||
-- 'ekaterinburg' -> 'Екатеринбург' 26770
|
||||
-- 'nizhniy_tagil' -> 'Нижний Тагил' 551 (148 сегодня в очереди геокода)
|
||||
-- 'kamensk-uralskiy' -> 'Каменск-Уральский' 244
|
||||
-- 'pervouralsk' -> 'Первоуральск' 95
|
||||
-- 'verhnyaya_pyshma' -> 'Верхняя Пышма' 21
|
||||
-- 'serov' -> 'Серов' 25
|
||||
-- ИТОГО 27706
|
||||
-- ⚠️ avito_slug у Каменска-Уральского — ЧЕРЕЗ ДЕФИС ('kamensk-uralskiy'), не
|
||||
-- через подчёркивание, в отличие от нашего внутреннего city_slug
|
||||
-- 'kamensk_uralskiy' (CITY_LOCATIONS ключ). У Верхней Пышмы наоборот —
|
||||
-- у Avito 'verhnyaya_pyshma' (kh -> h, БЕЗ 'k'), совпадает с
|
||||
-- CityLocation("verhnyaya_pyshma", ...).avito_slug в pipeline.py, но
|
||||
-- отличается от нашего внутреннего ключа 'verkhnyaya_pyshma' (с 'k').
|
||||
-- В фактических данных встретился ТОЛЬКО вариант 'verhnyaya_pyshma' — второй
|
||||
-- вариант написания в WHERE не нужен (дал бы 0 доп. строк).
|
||||
--
|
||||
-- ВНЕ SCOPE (сознательно не трогаем, обоснование):
|
||||
-- - Cian: хост НЕ индикатор города (ekb.cian.ru отдаёт областные объявления,
|
||||
-- включая тагильские, через тот же хост с параметром региона) — бэкфилл
|
||||
-- по хосту дал бы неверный результат.
|
||||
-- - Domclick: у объявлений без координат город не критичен (0 rows без
|
||||
-- lat), 13 строк на голом domclick.ru — отдельный разбор, не эта миграция.
|
||||
-- - Yandex: в URL (realty.yandex.ru/offer/<id>) города нет вовсе.
|
||||
-- - listings.region_code: у 16912 чужих-региона строк он неверный (стоит
|
||||
-- 66) — отдельный пункт issue #2604, ждёт решения владельца, здесь НЕ
|
||||
-- трогаем.
|
||||
-- - Слаги вне наших шести городов (1644 distinct на Avito, 16930 строк
|
||||
-- city IS NULL) остаются NULL — по ним отдельное решение владельца.
|
||||
--
|
||||
-- Idempotency:
|
||||
-- `WHERE city IS NULL` — не перетирает то, что уже проставил скрапер
|
||||
-- (write-path 196) или предыдущий прогон этой же миграции. Повторный
|
||||
-- прогон обновляет 0 строк (все затронутые строки уже НЕ city IS NULL).
|
||||
-- CASE ветки строго совпадают со списком в WHERE ... IN (...), поэтому
|
||||
-- для любой строки, прошедшей WHERE, CASE НЕ может вернуть NULL.
|
||||
--
|
||||
-- НЕ DDL — только UPDATE данных (колонка listings.city уже существует,
|
||||
-- миграция 196). Ни одна строка не удаляется и не деактивируется.
|
||||
--
|
||||
-- Dependencies: 196_listings_city.sql (колонка listings.city).
|
||||
|
||||
BEGIN;
|
||||
|
||||
UPDATE listings
|
||||
SET city = CASE substring(source_url from 'avito\.ru/([^/]+)/')
|
||||
WHEN 'ekaterinburg' THEN 'Екатеринбург'
|
||||
WHEN 'nizhniy_tagil' THEN 'Нижний Тагил'
|
||||
WHEN 'kamensk-uralskiy' THEN 'Каменск-Уральский'
|
||||
WHEN 'pervouralsk' THEN 'Первоуральск'
|
||||
WHEN 'verhnyaya_pyshma' THEN 'Верхняя Пышма'
|
||||
WHEN 'serov' THEN 'Серов'
|
||||
END
|
||||
WHERE source = 'avito'
|
||||
AND city IS NULL
|
||||
AND substring(source_url from 'avito\.ru/([^/]+)/') IN (
|
||||
'ekaterinburg',
|
||||
'nizhniy_tagil',
|
||||
'kamensk-uralskiy',
|
||||
'pervouralsk',
|
||||
'verhnyaya_pyshma',
|
||||
'serov'
|
||||
);
|
||||
|
||||
COMMIT;
|
||||
|
|
@ -0,0 +1,158 @@
|
|||
"""Static guards for migration 197 (issue #2594 шаг 3 — бэкфилл listings.city
|
||||
из слага города в Avito source_url для накопленных объявлений).
|
||||
|
||||
Прод применяет data/sql построчно строго (ON_ERROR_STOP). Полный DB-прогон
|
||||
требует живой БД; здесь фиксируем структурные инварианты, которые ГАРАНТИРУЮТ
|
||||
идемпотентность, скоуп (только Avito, только city IS NULL, только 6 наших
|
||||
городов) и НЕдеструктивность к самим listings-строкам по построению.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import re
|
||||
from pathlib import Path
|
||||
|
||||
_SQL_DIR = Path(__file__).resolve().parents[1] / "data" / "sql"
|
||||
_MIGRATION_197 = _SQL_DIR / "197_backfill_listings_city_from_url.sql"
|
||||
|
||||
|
||||
def _sql() -> str:
|
||||
return _MIGRATION_197.read_text(encoding="utf-8")
|
||||
|
||||
|
||||
def _executable_sql() -> str:
|
||||
"""SQL без построчных `--`-комментариев — только исполняемый код."""
|
||||
lines = []
|
||||
for raw in _sql().splitlines():
|
||||
code = raw.split("--", 1)[0]
|
||||
if code.strip():
|
||||
lines.append(code)
|
||||
return "\n".join(lines)
|
||||
|
||||
|
||||
def _flat(text: str) -> str:
|
||||
return re.sub(r"\s+", " ", text).strip().lower()
|
||||
|
||||
|
||||
def test_migration_197_exists() -> None:
|
||||
assert _MIGRATION_197.exists(), f"missing migration: {_MIGRATION_197}"
|
||||
|
||||
|
||||
def test_migration_197_is_transactional() -> None:
|
||||
sql = _sql()
|
||||
assert "BEGIN;" in sql
|
||||
assert "COMMIT;" in sql
|
||||
|
||||
|
||||
def test_migration_197_only_avito_city_null() -> None:
|
||||
"""WHERE ограничен source='avito' AND city IS NULL — не перетирает то, что
|
||||
уже проставил скрапер (196), не трогает Cian/Domclick/Yandex."""
|
||||
flat = _flat(_executable_sql())
|
||||
assert "where source = 'avito'" in flat
|
||||
assert "and city is null" in flat
|
||||
|
||||
|
||||
def test_migration_197_covers_exactly_six_cities() -> None:
|
||||
"""CASE и WHERE ... IN покрывают ровно наши шесть городов Свердловской
|
||||
обл. — ни больше (не расползаемся на чужие регионы), ни меньше."""
|
||||
flat = _flat(_executable_sql())
|
||||
expected_pairs = {
|
||||
"'ekaterinburg'": "екатеринбург",
|
||||
"'nizhniy_tagil'": "нижний тагил",
|
||||
"'kamensk-uralskiy'": "каменск-уральский",
|
||||
"'pervouralsk'": "первоуральск",
|
||||
"'verhnyaya_pyshma'": "верхняя пышма",
|
||||
"'serov'": "серов",
|
||||
}
|
||||
for slug, _city_lower in expected_pairs.items():
|
||||
assert slug in flat, f"missing avito slug branch: {slug}"
|
||||
# Ровно 6 веток WHEN в CASE (по числу городов).
|
||||
assert flat.count(" when ") == len(expected_pairs)
|
||||
|
||||
|
||||
def test_migration_197_kamensk_slug_uses_dash_not_underscore() -> None:
|
||||
"""Avito отдаёт 'kamensk-uralskiy' (дефис) — НЕ наш внутренний city_slug
|
||||
'kamensk_uralskiy' (подчёркивание, CITY_LOCATIONS ключ в pipeline.py).
|
||||
Регресс на подчёркивание означало бы 0 подхваченных строк на проде."""
|
||||
flat = _flat(_executable_sql())
|
||||
assert "'kamensk-uralskiy'" in flat
|
||||
assert "'kamensk_uralskiy'" not in flat
|
||||
|
||||
|
||||
def test_migration_197_pyshma_slug_matches_avito_not_internal_key() -> None:
|
||||
"""Avito слаг — 'verhnyaya_pyshma' (без 'k'), а не наш внутренний ключ
|
||||
'verkhnyaya_pyshma' (с 'k', CITY_DISPLAY_NAMES/CITY_LOCATIONS в
|
||||
pipeline.py). На проде встретился только вариант без 'k' — второй сюда
|
||||
сознательно не добавлен (см. заголовок миграции)."""
|
||||
flat = _flat(_executable_sql())
|
||||
assert "'verhnyaya_pyshma'" in flat
|
||||
assert "'verkhnyaya_pyshma'" not in flat
|
||||
|
||||
|
||||
def test_migration_197_city_names_match_pipeline_display_names() -> None:
|
||||
"""Человекочитаемые названия городов побайтно совпадают с
|
||||
CITY_DISPLAY_NAMES / EKATERINBURG_CITY_NAME в scraper_kit.orchestration
|
||||
.pipeline — иначе один и тот же город расщепится на две разные метки
|
||||
(старые backfilled-строки vs новые, проставленные скрапером)."""
|
||||
pipeline_path = (
|
||||
Path(__file__).resolve().parents[2]
|
||||
/ "packages"
|
||||
/ "scraper-kit"
|
||||
/ "src"
|
||||
/ "scraper_kit"
|
||||
/ "orchestration"
|
||||
/ "pipeline.py"
|
||||
)
|
||||
pipeline_src = pipeline_path.read_text(encoding="utf-8")
|
||||
|
||||
sql = _sql()
|
||||
expected_names = [
|
||||
"Екатеринбург",
|
||||
"Нижний Тагил",
|
||||
"Каменск-Уральский",
|
||||
"Первоуральск",
|
||||
"Верхняя Пышма",
|
||||
"Серов",
|
||||
]
|
||||
for name in expected_names:
|
||||
assert name in sql, f"missing display name in migration: {name}"
|
||||
assert name in pipeline_src, (
|
||||
f"display name {name!r} in migration 197 не найден в pipeline.py "
|
||||
"CITY_DISPLAY_NAMES/EKATERINBURG_CITY_NAME — риск расщепления "
|
||||
"одного города на две метки"
|
||||
)
|
||||
|
||||
|
||||
def test_migration_197_no_ddl() -> None:
|
||||
"""Только UPDATE данных — колонка listings.city уже существует (196),
|
||||
никакого ALTER/CREATE/DROP здесь быть не должно."""
|
||||
flat = _flat(_executable_sql())
|
||||
assert "alter table" not in flat
|
||||
assert "create table" not in flat
|
||||
assert "drop table" not in flat
|
||||
assert flat.count("update listings") == 1
|
||||
|
||||
|
||||
def test_migration_197_no_destructive_ddl() -> None:
|
||||
"""Миграция не должна содержать DROP TABLE / TRUNCATE / DELETE."""
|
||||
flat = _flat(_executable_sql())
|
||||
assert "drop table" not in flat
|
||||
assert "truncate" not in flat
|
||||
assert "delete from" not in flat
|
||||
|
||||
|
||||
def test_migration_197_does_not_touch_other_sources_or_region_code() -> None:
|
||||
"""Явно вне scope (#2601/#2604): cian/yandex/domclick и region_code не
|
||||
упоминаются в исполняемом SQL этой миграции."""
|
||||
flat = _flat(_executable_sql())
|
||||
assert "cian" not in flat
|
||||
assert "yandex" not in flat
|
||||
assert "domclick" not in flat
|
||||
assert "region_code" not in flat
|
||||
|
||||
|
||||
def test_migration_197_no_psycopg_trap() -> None:
|
||||
"""Никаких :param::type — psycopg v3 требует CAST(... AS type) (не
|
||||
применимо в чистом .sql без bind params, но проверяем на регресс
|
||||
copy-paste из Python-кода)."""
|
||||
assert not re.search(r":\w+::", _sql())
|
||||
Loading…
Add table
Reference in a new issue