fix(nspd-sync): address PR #111 auto-review M1-M5+M7 findings
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.
This commit is contained in:
parent
9b2a289e83
commit
aca4b443aa
3 changed files with 10 additions and 10 deletions
|
|
@ -302,6 +302,9 @@ def _resume_zombie_runs(sender=None, **_kwargs) -> None:
|
||||||
)
|
)
|
||||||
)
|
)
|
||||||
_db.commit()
|
_db.commit()
|
||||||
|
except Exception:
|
||||||
|
_db.rollback()
|
||||||
|
raise
|
||||||
finally:
|
finally:
|
||||||
_db.close()
|
_db.close()
|
||||||
except Exception as _e:
|
except Exception as _e:
|
||||||
|
|
|
||||||
|
|
@ -18,6 +18,7 @@ UPSERT pattern:
|
||||||
|
|
||||||
from __future__ import annotations
|
from __future__ import annotations
|
||||||
|
|
||||||
|
import datetime as _dt
|
||||||
import json
|
import json
|
||||||
import logging
|
import logging
|
||||||
import time
|
import time
|
||||||
|
|
@ -179,7 +180,7 @@ def _upsert_dump(
|
||||||
"risks_count": _build_risks_count(dump),
|
"risks_count": _build_risks_count(dump),
|
||||||
"total_features": dump.total_features,
|
"total_features": dump.total_features,
|
||||||
"features_json": json.dumps(features_json or [], ensure_ascii=False),
|
"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,
|
"fetched_at_utc": dump.fetched_at_utc,
|
||||||
"harvest_duration_ms": duration_ms,
|
"harvest_duration_ms": duration_ms,
|
||||||
"harvest_error": harvest_error,
|
"harvest_error": harvest_error,
|
||||||
|
|
@ -187,8 +188,6 @@ def _upsert_dump(
|
||||||
}
|
}
|
||||||
else:
|
else:
|
||||||
# Error-only row: нулевые счётчики, harvest_error filled
|
# Error-only row: нулевые счётчики, harvest_error filled
|
||||||
import datetime as _dt
|
|
||||||
|
|
||||||
params = {
|
params = {
|
||||||
"quarter_cad": quarter_cad,
|
"quarter_cad": quarter_cad,
|
||||||
"geom_json": None,
|
"geom_json": None,
|
||||||
|
|
@ -205,7 +204,7 @@ def _upsert_dump(
|
||||||
"risks_count": 0,
|
"risks_count": 0,
|
||||||
"total_features": 0,
|
"total_features": 0,
|
||||||
"features_json": "[]",
|
"features_json": "[]",
|
||||||
"layers_fetched": "{}",
|
"layers_fetched": [],
|
||||||
"fetched_at_utc": _dt.datetime.now(_dt.UTC).isoformat(),
|
"fetched_at_utc": _dt.datetime.now(_dt.UTC).isoformat(),
|
||||||
"harvest_duration_ms": duration_ms,
|
"harvest_duration_ms": duration_ms,
|
||||||
"harvest_error": harvest_error,
|
"harvest_error": harvest_error,
|
||||||
|
|
@ -214,6 +213,9 @@ def _upsert_dump(
|
||||||
|
|
||||||
db.execute(_UPSERT_SQL, params)
|
db.execute(_UPSERT_SQL, params)
|
||||||
db.commit()
|
db.commit()
|
||||||
|
except Exception:
|
||||||
|
db.rollback()
|
||||||
|
raise
|
||||||
finally:
|
finally:
|
||||||
db.close()
|
db.close()
|
||||||
|
|
||||||
|
|
@ -360,8 +362,7 @@ def harvest_stale_quarters(
|
||||||
"""
|
"""
|
||||||
SELECT cad_number
|
SELECT cad_number
|
||||||
FROM cad_quarters_geom
|
FROM cad_quarters_geom
|
||||||
WHERE region_code = :rc
|
WHERE cad_number NOT IN (
|
||||||
AND cad_number NOT IN (
|
|
||||||
SELECT quarter_cad
|
SELECT quarter_cad
|
||||||
FROM nspd_quarter_dumps
|
FROM nspd_quarter_dumps
|
||||||
WHERE region_code = :rc
|
WHERE region_code = :rc
|
||||||
|
|
|
||||||
|
|
@ -296,8 +296,6 @@ def test_harvest_stale_quarters_fanout(
|
||||||
# Mock DB session
|
# Mock DB session
|
||||||
mock_db = MagicMock()
|
mock_db = MagicMock()
|
||||||
mock_session_cls.return_value = mock_db
|
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]
|
mock_db.execute.return_value.all.return_value = [(c,) for c in stale_cads]
|
||||||
|
|
||||||
# harvest_quarter.apply_async — проверяем что вызван для каждого cad
|
# harvest_quarter.apply_async — проверяем что вызван для каждого cad
|
||||||
|
|
@ -324,8 +322,6 @@ def test_harvest_stale_quarters_empty(
|
||||||
"""Нет stale кварталов → 0 enqueued, apply_async не вызван."""
|
"""Нет stale кварталов → 0 enqueued, apply_async не вызван."""
|
||||||
mock_db = MagicMock()
|
mock_db = MagicMock()
|
||||||
mock_session_cls.return_value = mock_db
|
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_db.execute.return_value.all.return_value = []
|
||||||
|
|
||||||
mock_task.apply_async = MagicMock()
|
mock_task.apply_async = MagicMock()
|
||||||
|
|
|
||||||
Loading…
Add table
Reference in a new issue