From aca4b443aae88604dd64a169492bb724f74b8e43 Mon Sep 17 00:00:00 2001 From: lekss361 Date: Tue, 12 May 2026 18:43:02 +0300 Subject: [PATCH] fix(nspd-sync): address PR #111 auto-review M1-M5+M7 findings MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit M1 (runtime bug): remove WHERE region_code from outer cad_quarters_geom SELECT in harvest_stale_quarters — column does not exist in prod schema (cad_number, geom, raw_props, fetched_at, source). Would crash on first beat tick. Inner nspd_quarter_dumps subquery keeps region_code filter unchanged. M2 (correctness): _upsert_dump теперь делает db.rollback() в except перед db.close() — prevents PendingRollbackError на следующем session use, если PostGIS отбракует geometry или JSON cast failure. M3 (low): worker_ready breadcrumb INSERT тоже теперь имеет rollback — consistency с остальными session usages в celery_app.py. M4 (style): layers_fetched — заменил manual string concat '{' + ','.join(...) + '}' на native Python list. psycopg v3 сам сериализует list → text[]. Защита от future layer names с запятыми/фигурными скобками. M5 (style): import datetime as _dt поднят из else branch в top-level imports. M7 (test cleanup): удалил dead __enter__/__exit__ mock setup в test_harvest_stale_quarters_fanout / _empty. harvest_stale_quarters не использует session как context manager — moc'и no-op. 14/14 tests pass. ruff/format clean. Per auto-review on 9b2a289. --- backend/app/workers/celery_app.py | 3 +++ backend/app/workers/tasks/nspd_sync.py | 13 +++++++------ backend/tests/test_nspd_sync.py | 4 ---- 3 files changed, 10 insertions(+), 10 deletions(-) diff --git a/backend/app/workers/celery_app.py b/backend/app/workers/celery_app.py index 83139521..afa18809 100644 --- a/backend/app/workers/celery_app.py +++ b/backend/app/workers/celery_app.py @@ -302,6 +302,9 @@ def _resume_zombie_runs(sender=None, **_kwargs) -> None: ) ) _db.commit() + except Exception: + _db.rollback() + raise finally: _db.close() except Exception as _e: diff --git a/backend/app/workers/tasks/nspd_sync.py b/backend/app/workers/tasks/nspd_sync.py index 44d74353..af6567f1 100644 --- a/backend/app/workers/tasks/nspd_sync.py +++ b/backend/app/workers/tasks/nspd_sync.py @@ -18,6 +18,7 @@ UPSERT pattern: from __future__ import annotations +import datetime as _dt import json import logging import time @@ -179,7 +180,7 @@ def _upsert_dump( "risks_count": _build_risks_count(dump), "total_features": dump.total_features, "features_json": json.dumps(features_json or [], ensure_ascii=False), - "layers_fetched": "{" + ",".join(dump.layers_fetched) + "}", + "layers_fetched": list(dump.layers_fetched), "fetched_at_utc": dump.fetched_at_utc, "harvest_duration_ms": duration_ms, "harvest_error": harvest_error, @@ -187,8 +188,6 @@ def _upsert_dump( } else: # Error-only row: нулевые счётчики, harvest_error filled - import datetime as _dt - params = { "quarter_cad": quarter_cad, "geom_json": None, @@ -205,7 +204,7 @@ def _upsert_dump( "risks_count": 0, "total_features": 0, "features_json": "[]", - "layers_fetched": "{}", + "layers_fetched": [], "fetched_at_utc": _dt.datetime.now(_dt.UTC).isoformat(), "harvest_duration_ms": duration_ms, "harvest_error": harvest_error, @@ -214,6 +213,9 @@ def _upsert_dump( db.execute(_UPSERT_SQL, params) db.commit() + except Exception: + db.rollback() + raise finally: db.close() @@ -360,8 +362,7 @@ def harvest_stale_quarters( """ SELECT cad_number FROM cad_quarters_geom - WHERE region_code = :rc - AND cad_number NOT IN ( + WHERE cad_number NOT IN ( SELECT quarter_cad FROM nspd_quarter_dumps WHERE region_code = :rc diff --git a/backend/tests/test_nspd_sync.py b/backend/tests/test_nspd_sync.py index 40c712fd..8b077cf6 100644 --- a/backend/tests/test_nspd_sync.py +++ b/backend/tests/test_nspd_sync.py @@ -296,8 +296,6 @@ def test_harvest_stale_quarters_fanout( # Mock DB session mock_db = MagicMock() mock_session_cls.return_value = mock_db - mock_db.__enter__ = MagicMock(return_value=mock_db) - mock_db.__exit__ = MagicMock(return_value=False) mock_db.execute.return_value.all.return_value = [(c,) for c in stale_cads] # harvest_quarter.apply_async — проверяем что вызван для каждого cad @@ -324,8 +322,6 @@ def test_harvest_stale_quarters_empty( """Нет stale кварталов → 0 enqueued, apply_async не вызван.""" mock_db = MagicMock() mock_session_cls.return_value = mock_db - mock_db.__enter__ = MagicMock(return_value=mock_db) - mock_db.__exit__ = MagicMock(return_value=False) mock_db.execute.return_value.all.return_value = [] mock_task.apply_async = MagicMock()