From d5f1354f1d7b605da504832c7d51431e2f2c4bb2 Mon Sep 17 00:00:00 2001 From: Joypal Navaneeth Date: Tue, 1 Sep 2026 13:06:03 +0530 Subject: [PATCH] perf: get_health_summary uses one query instead of 4 per source --- src/tendertracker/pipeline/health.py | 35 ++++++----- tests/test_health.py | 90 ++++++++++++++++++++++++++++ 2 files changed, 110 insertions(+), 15 deletions(-) create mode 100644 tests/test_health.py diff --git a/src/tendertracker/pipeline/health.py b/src/tendertracker/pipeline/health.py index c84fc8e..a1896bb 100644 --- a/src/tendertracker/pipeline/health.py +++ b/src/tendertracker/pipeline/health.py @@ -19,28 +19,33 @@ class SourceHealth: def get_health_summary(session: Session) -> list[SourceHealth]: """Per-source rollup — the "at a glance" view an ops user actually wants, versus `status`'s flat recent-runs-across-all-sources list where - a quiet source's last run can get buried by a noisy one.""" - sources = sorted(row[0] for row in session.query(SyncLog.source).distinct().all()) + a quiet source's last run can get buried by a noisy one. + + One query fetches every SyncLog row (newest first), then aggregates per + source in Python — previously 4 separate queries per source (4N+1 + total), fine at this project's real scale (a handful of sources) but a + real inefficiency if it ever needs to track many. + """ + logs = session.query(SyncLog).order_by(SyncLog.started_at.desc()).all() + + by_source: dict[str, list[SyncLog]] = {} + for log in logs: + by_source.setdefault(log.source, []).append(log) summary = [] - for source in sources: - last_run = session.query(SyncLog).filter_by(source=source).order_by(SyncLog.started_at.desc()).first() - last_success = ( - session.query(SyncLog) - .filter_by(source=source, status="success") - .order_by(SyncLog.started_at.desc()) - .first() - ) - error_count = session.query(SyncLog).filter(SyncLog.source == source, SyncLog.error_message.isnot(None)).count() - total_runs = session.query(SyncLog).filter_by(source=source).count() + for source in sorted(by_source): + source_logs = by_source[source] # already newest-first + last_run = source_logs[0] + last_success = next((log for log in source_logs if log.status == "success"), None) + error_count = sum(1 for log in source_logs if log.error_message is not None) summary.append( SourceHealth( source=source, - last_run_at=last_run.started_at if last_run else None, - last_status=last_run.status if last_run else None, + last_run_at=last_run.started_at, + last_status=last_run.status, last_success_at=last_success.started_at if last_success else None, error_count=error_count, - total_runs=total_runs, + total_runs=len(source_logs), ) ) return summary diff --git a/tests/test_health.py b/tests/test_health.py new file mode 100644 index 0000000..1d5fad7 --- /dev/null +++ b/tests/test_health.py @@ -0,0 +1,90 @@ +from tendertracker.pipeline.health import get_health_summary, get_recent_errors +from tendertracker.storage.models import SyncLog, utcnow + + +def _log(source: str, status: str, error_message: str | None = None) -> SyncLog: + return SyncLog(source=source, started_at=utcnow(), finished_at=utcnow(), status=status, error_message=error_message) + + +def test_empty_db_returns_empty_summary(db_session): + assert get_health_summary(db_session) == [] + + +def test_single_source_single_run(db_session): + db_session.add(_log("sandbox", "success")) + db_session.commit() + + summary = get_health_summary(db_session) + assert len(summary) == 1 + s = summary[0] + assert s.source == "sandbox" + assert s.last_status == "success" + assert s.last_success_at is not None + assert s.error_count == 0 + assert s.total_runs == 1 + + +def test_last_success_skips_a_more_recent_failure(db_session): + """The whole point of tracking last_success separately from last_run — + a source that's currently failing should still show when it last + actually worked, not just 'never'.""" + import time + + db_session.add(_log("sandbox", "success")) + db_session.commit() + time.sleep(0.01) + db_session.add(_log("sandbox", "failed", error_message="boom")) + db_session.commit() + + summary = get_health_summary(db_session) + s = summary[0] + assert s.last_status == "failed" # most recent run failed + assert s.last_success_at is not None # but a success exists earlier + assert s.error_count == 1 + assert s.total_runs == 2 + + +def test_never_succeeded_reports_none(db_session): + db_session.add(_log("sandbox", "failed", error_message="boom")) + db_session.commit() + + s = get_health_summary(db_session)[0] + assert s.last_success_at is None + + +def test_multiple_sources_kept_separate_and_sorted(db_session): + db_session.add(_log("zzz-source", "success")) + db_session.add(_log("aaa-source", "success")) + db_session.commit() + + summary = get_health_summary(db_session) + assert [s.source for s in summary] == ["aaa-source", "zzz-source"] + + +def test_error_count_only_counts_rows_with_error_message(db_session): + db_session.add(_log("sandbox", "success")) + db_session.add(_log("sandbox", "dry-run")) + db_session.add(_log("sandbox", "failed", error_message="boom")) + db_session.commit() + + s = get_health_summary(db_session)[0] + assert s.error_count == 1 + assert s.total_runs == 3 + + +def test_get_recent_errors_only_returns_rows_with_error_message(db_session): + db_session.add(_log("sandbox", "success")) + db_session.add(_log("sandbox", "failed", error_message="boom")) + db_session.commit() + + errors = get_recent_errors(db_session) + assert len(errors) == 1 + assert errors[0].error_message == "boom" + + +def test_get_recent_errors_respects_limit(db_session): + for i in range(5): + db_session.add(_log("sandbox", "failed", error_message=f"error {i}")) + db_session.commit() + + assert len(get_recent_errors(db_session, limit=2)) == 2