From 564834a2a6b9ec16141a7dbfd9e18ce0a68bba9e Mon Sep 17 00:00:00 2001 From: malice Date: Fri, 22 May 2026 21:38:40 -0600 Subject: [PATCH] =?UTF-8?q?navi-geo:=20revert=20netsyms.health()=20wiring?= =?UTF-8?q?=20=E2=80=94=20was=20a=20cold-start=20footgun=20(#8)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PR #6 round-1 fixup #3 wired netsyms.health() (COUNT + DISTINCT on 35 GB) into _netsyms_fs_entry, adding >3s latency to cold admin-info calls. navi-admin's fleet fan-out (3s timeout) caught it after #7 deploy. Reverting to the cheap _file_entry shape; deleting health() per the no-dead-code rule (the original fallback option). Co-authored-by: zvx-echo6 Co-authored-by: Claude Opus 4.7 (1M context) --- backend/services/navi_geo/admin.py | 21 +------- backend/services/navi_geo/netsyms.py | 52 ++----------------- .../services/navi_geo/tests/test_geocode.py | 26 ---------- 3 files changed, 4 insertions(+), 95 deletions(-) diff --git a/backend/services/navi_geo/admin.py b/backend/services/navi_geo/admin.py index 83ec61a..83ccd27 100644 --- a/backend/services/navi_geo/admin.py +++ b/backend/services/navi_geo/admin.py @@ -16,7 +16,6 @@ from flask import Blueprint, jsonify, current_app from shared.auth import require_auth from shared.admin_info import build_info_response -from . import netsyms from .geocode import photon_url from .landclass_client import landclass_url from .netsyms import db_path as netsyms_db_path @@ -76,24 +75,6 @@ def _file_entry(path): } -def _netsyms_fs_entry(path): - """netsyms filesystem entry enriched with netsyms.health() — row count, file - size, and indexed countries on top of the standard path/exists/readable. - - health() degrades gracefully (ok:False, zeros) when the DB is absent, so this - never raises. row_count is cached after the first call inside netsyms; the - DISTINCT-country query runs per call but admin-info is auth-gated + rare.""" - entry = _file_entry(path) - h = netsyms.health() - entry.update({ - 'ok': h['ok'], - 'row_count': h['row_count'], - 'file_size_bytes': h['file_size_bytes'], - 'indexed_countries': h['indexed_countries'], - }) - return entry - - @bp.route('/api/admin/navi-geo/info') @require_auth def navi_geo_info(): @@ -123,7 +104,7 @@ def navi_geo_info(): _landclass_dependency(), ], filesystem=[ - _netsyms_fs_entry(netsyms_db), + _file_entry(netsyms_db), _file_entry(timezone_db), _file_entry(dem_file), _file_entry(address_book_path()), diff --git a/backend/services/navi_geo/netsyms.py b/backend/services/navi_geo/netsyms.py index 5243169..319dc40 100755 --- a/backend/services/navi_geo/netsyms.py +++ b/backend/services/navi_geo/netsyms.py @@ -27,13 +27,12 @@ def db_path(): _conn = None _lock = threading.Lock() -_cached_row_count = None def reset_conn(): - """Drop the cached connection + row count so the next call reopens (env may - have changed). Used per app instance / per test.""" - global _conn, _cached_row_count + """Drop the cached connection so the next call reopens (env may have + changed). Used per app instance / per test.""" + global _conn with _lock: if _conn is not None: try: @@ -41,7 +40,6 @@ def reset_conn(): except Exception: pass _conn = None - _cached_row_count = None # US states + DC + territories, CA provinces, for free-text parsing _STATE_CODES = { @@ -204,47 +202,3 @@ def lookup_by_zipcode(zipcode, limit=100): results = [_row_to_dict(r) for r in rows] logger.debug("lookup_by_zipcode(%s) → %d results", zipcode, len(results)) return results - - -def health(): - """Health check with cached row count.""" - global _cached_row_count - - try: - file_size = os.path.getsize(db_path()) - except OSError: - return {'ok': False, 'row_count': 0, 'file_size_bytes': 0, - 'indexed_countries': []} - - try: - conn = _get_conn() - except Exception: - return {'ok': False, 'row_count': 0, 'file_size_bytes': file_size, - 'indexed_countries': []} - - if _cached_row_count is None: - with _lock: - if _cached_row_count is None: - try: - row = conn.execute( - "SELECT COUNT(*) AS cnt FROM addresses" - ).fetchone() - _cached_row_count = row['cnt'] - except sqlite3.Error: - _cached_row_count = 0 - - with _lock: - try: - rows = conn.execute( - "SELECT DISTINCT country FROM addresses" - ).fetchall() - countries = sorted(r['country'] for r in rows) - except sqlite3.Error: - countries = [] - - return { - 'ok': True, - 'row_count': _cached_row_count, - 'file_size_bytes': file_size, - 'indexed_countries': countries, - } diff --git a/backend/services/navi_geo/tests/test_geocode.py b/backend/services/navi_geo/tests/test_geocode.py index 2bd1c0a..78396ea 100644 --- a/backend/services/navi_geo/tests/test_geocode.py +++ b/backend/services/navi_geo/tests/test_geocode.py @@ -215,29 +215,3 @@ def test_admin_info_has_no_secrets_and_two_probes(monkeypatch): def test_admin_info_requires_auth(): client = create_app().test_client() assert client.get('/api/admin/navi-geo/info').status_code == 401 - - -def test_admin_info_netsyms_entry_enriched_with_health(tmp_path, monkeypatch): - # netsyms.health() is wired into the netsyms filesystem entry (review fix #3): - # the entry carries row_count / file_size_bytes / indexed_countries on top of - # the standard path/exists/readable. Use a tiny real sqlite so health() runs. - import sqlite3 - db = tmp_path / 'netsyms.sqlite' - con = sqlite3.connect(db) - con.execute('CREATE TABLE addresses (country TEXT)') - con.executemany('INSERT INTO addresses (country) VALUES (?)', - [('US',), ('US',), ('CA',)]) - con.commit() - con.close() - monkeypatch.setenv('NAVI_NETSYMS_DB', str(db)) - - client = create_app().test_client() # reset_conn() picks up the new path - resp = client.get('/api/admin/navi-geo/info', - headers={'X-Authentik-Username': 'matt'}) - assert resp.status_code == 200 - fs = resp.get_json()['filesystem'] - netsyms_entry = next(e for e in fs if e['path'] == str(db)) - assert netsyms_entry['ok'] is True - assert netsyms_entry['row_count'] == 3 - assert netsyms_entry['file_size_bytes'] > 0 - assert set(netsyms_entry['indexed_countries']) == {'US', 'CA'}