Fix compare place names (Nominatim burst) + weekly duplicate location name (#29)
Compare was the only page loading several cells at once, so its concurrent reverse-geocode calls burst past Nominatim's ~1 req/sec limit, got rate-limited, and cached a null label — leaving those locations stuck on bare coordinates. - climate.py: serialize + rate-limit reverse_geocode behind a lock (>=1.1s between Nominatim calls, re-checking the cache under the lock), so concurrent callers resolve reliably instead of bursting. - store.py: shorten the reverse-geocode miss TTL 1 day -> 1 hour so a transient null retries soon; add a `cache` flag to put_payload. - app.py: don't persist a calendar payload whose place failed to resolve, so the coordinates fallback can't stick for the life of the token. Weekly showed the place name twice (top label beside the Find button AND the results <h2>). Keep the <h2> (it carries the coords + climatology context) as the single name display; the top label now only holds the transient coordinates written on selection and is hidden once the named results render. Claude-Session: https://claude.ai/code/session_01B3Q2EkHHnTUX2BfhV5q1Zc Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
parent
e01d2e0eb8
commit
7358d9eb48
3 changed files with 63 additions and 31 deletions
8
app.py
8
app.py
|
|
@ -421,7 +421,13 @@ def api_calendar(
|
||||||
full = not cache_meta.get("cached", False)
|
full = not cache_meta.get("cached", False)
|
||||||
run.set(run_type="full" if full else "partial",
|
run.set(run_type="full" if full else "partial",
|
||||||
history_source="fetch" if full else "cache")
|
history_source="fetch" if full else "cache")
|
||||||
return _json_response(store.put_payload("calendar", cell["id"], key, token, payload), etag)
|
# Don't persist a payload whose place failed to resolve (a transient reverse-
|
||||||
|
# geocode miss) — otherwise the bare-coordinates fallback would stick for the
|
||||||
|
# life of the token. Compare loads several cells at once, so this is where a
|
||||||
|
# miss is most likely; leaving it uncached lets the next request retry.
|
||||||
|
return _json_response(
|
||||||
|
store.put_payload("calendar", cell["id"], key, token, payload,
|
||||||
|
cache=place is not None), etag)
|
||||||
|
|
||||||
|
|
||||||
def api_day(
|
def api_day(
|
||||||
|
|
|
||||||
75
climate.py
75
climate.py
|
|
@ -499,6 +499,14 @@ def _load_recent_forecast(cell: dict) -> pd.DataFrame:
|
||||||
|
|
||||||
_REVGEO_CACHE: dict[str, str | None] = {}
|
_REVGEO_CACHE: dict[str, str | None] = {}
|
||||||
|
|
||||||
|
# Nominatim's usage policy allows ~1 request/second and rejects bursts. The compare
|
||||||
|
# page loads several locations at once, so serialize the reverse lookups here and
|
||||||
|
# space them out — otherwise the burst is rate-limited, a null label gets cached, and
|
||||||
|
# those locations are left showing bare coordinates.
|
||||||
|
_REVGEO_LOCK = threading.Lock()
|
||||||
|
_REVGEO_MIN_INTERVAL = 1.1 # seconds between successive Nominatim reverse calls
|
||||||
|
_revgeo_last = 0.0
|
||||||
|
|
||||||
|
|
||||||
def reverse_geocode_cached(lat: float, lon: float) -> tuple[bool, str | None]:
|
def reverse_geocode_cached(lat: float, lon: float) -> tuple[bool, str | None]:
|
||||||
"""(found, label) from the in-memory + SQLite revgeo caches only — never calls
|
"""(found, label) from the in-memory + SQLite revgeo caches only — never calls
|
||||||
|
|
@ -523,34 +531,47 @@ def reverse_geocode(lat: float, lon: float) -> str | None:
|
||||||
found, label = reverse_geocode_cached(lat, lon)
|
found, label = reverse_geocode_cached(lat, lon)
|
||||||
if found:
|
if found:
|
||||||
return label
|
return label
|
||||||
label = None
|
# Serialize + rate-limit the upstream call. Concurrent callers (the compare page
|
||||||
try:
|
# loads several locations at once) would otherwise burst past Nominatim's ~1/sec
|
||||||
r = _request(
|
# limit and get rate-limited, caching a null label. Under the lock we re-check the
|
||||||
"https://nominatim.openstreetmap.org/reverse",
|
# cache (a peer may have just resolved this cell) and space successive calls out.
|
||||||
# zoom 14 resolves to the suburb/neighbourhood level so we can lead
|
global _revgeo_last
|
||||||
# with it when OSM has one (zoom 10 only ever returns the city).
|
with _REVGEO_LOCK:
|
||||||
{"lat": lat, "lon": lon, "format": "jsonv2", "zoom": 14,
|
found, label = reverse_geocode_cached(lat, lon)
|
||||||
"addressdetails": 1},
|
if found:
|
||||||
15,
|
return label
|
||||||
phase="reverse_geocode",
|
wait = _REVGEO_MIN_INTERVAL - (time.monotonic() - _revgeo_last)
|
||||||
headers={"User-Agent": "Thermograph/0.1 (local weather grading app)"},
|
if wait > 0:
|
||||||
)
|
time.sleep(wait)
|
||||||
a = r.json().get("address", {}) or {}
|
|
||||||
# Lead with the neighbourhood when available, then the city, then region.
|
|
||||||
neighborhood = (a.get("neighbourhood") or a.get("suburb")
|
|
||||||
or a.get("quarter") or a.get("city_district")
|
|
||||||
or a.get("borough"))
|
|
||||||
city = (a.get("city") or a.get("town") or a.get("village")
|
|
||||||
or a.get("hamlet") or a.get("county"))
|
|
||||||
region = a.get("state") or a.get("province") or a.get("region")
|
|
||||||
# dict.fromkeys drops duplicates (e.g. neighbourhood == city) in order.
|
|
||||||
parts = dict.fromkeys(p for p in (neighborhood, city, region) if p)
|
|
||||||
label = ", ".join(parts) or None
|
|
||||||
except Exception: # noqa: BLE001 - reverse geocoding is a nicety, never fatal
|
|
||||||
label = None
|
label = None
|
||||||
_REVGEO_CACHE[key] = label
|
try:
|
||||||
store.put_revgeo(key, label) # survive restarts (a None label is retried daily)
|
r = _request(
|
||||||
return label
|
"https://nominatim.openstreetmap.org/reverse",
|
||||||
|
# zoom 14 resolves to the suburb/neighbourhood level so we can lead
|
||||||
|
# with it when OSM has one (zoom 10 only ever returns the city).
|
||||||
|
{"lat": lat, "lon": lon, "format": "jsonv2", "zoom": 14,
|
||||||
|
"addressdetails": 1},
|
||||||
|
15,
|
||||||
|
phase="reverse_geocode",
|
||||||
|
headers={"User-Agent": "Thermograph/0.1 (local weather grading app)"},
|
||||||
|
)
|
||||||
|
a = r.json().get("address", {}) or {}
|
||||||
|
# Lead with the neighbourhood when available, then the city, then region.
|
||||||
|
neighborhood = (a.get("neighbourhood") or a.get("suburb")
|
||||||
|
or a.get("quarter") or a.get("city_district")
|
||||||
|
or a.get("borough"))
|
||||||
|
city = (a.get("city") or a.get("town") or a.get("village")
|
||||||
|
or a.get("hamlet") or a.get("county"))
|
||||||
|
region = a.get("state") or a.get("province") or a.get("region")
|
||||||
|
# dict.fromkeys drops duplicates (e.g. neighbourhood == city) in order.
|
||||||
|
parts = dict.fromkeys(p for p in (neighborhood, city, region) if p)
|
||||||
|
label = ", ".join(parts) or None
|
||||||
|
except Exception: # noqa: BLE001 - reverse geocoding is a nicety, never fatal
|
||||||
|
label = None
|
||||||
|
_revgeo_last = time.monotonic()
|
||||||
|
_REVGEO_CACHE[key] = label
|
||||||
|
store.put_revgeo(key, label) # survive restarts (a None label retries after its TTL)
|
||||||
|
return label
|
||||||
|
|
||||||
|
|
||||||
def geocode(name: str, count: int = 5) -> list[dict]:
|
def geocode(name: str, count: int = 5) -> list[dict]:
|
||||||
|
|
|
||||||
11
store.py
11
store.py
|
|
@ -92,16 +92,21 @@ def get_json(kind: str, cell_id: str, key: str, token: str) -> dict | None:
|
||||||
return json.loads(body) if body is not None else None
|
return json.loads(body) if body is not None else None
|
||||||
|
|
||||||
|
|
||||||
def put_payload(kind: str, cell_id: str, key: str, token: str, payload: dict) -> bytes:
|
def put_payload(kind: str, cell_id: str, key: str, token: str, payload: dict,
|
||||||
|
cache: bool = True) -> bytes:
|
||||||
"""Encode ``payload`` to compact JSON, persist it, and return the encoded bytes
|
"""Encode ``payload`` to compact JSON, persist it, and return the encoded bytes
|
||||||
(so the caller can serve the exact same bytes without re-serializing).
|
(so the caller can serve the exact same bytes without re-serializing).
|
||||||
|
|
||||||
|
Pass ``cache=False`` to serve the payload without persisting it — used when a
|
||||||
|
best-effort field (the reverse-geocoded place name) failed to resolve, so the
|
||||||
|
incomplete result isn't cached for the life of the token.
|
||||||
|
|
||||||
allow_nan=False mirrors Starlette's JSONResponse: a NaN slipping through
|
allow_nan=False mirrors Starlette's JSONResponse: a NaN slipping through
|
||||||
grading should fail loudly here, not be cached as JS-unparseable `NaN`."""
|
grading should fail loudly here, not be cached as JS-unparseable `NaN`."""
|
||||||
body = json.dumps(payload, default=str, allow_nan=False,
|
body = json.dumps(payload, default=str, allow_nan=False,
|
||||||
separators=(",", ":")).encode()
|
separators=(",", ":")).encode()
|
||||||
conn = _conn()
|
conn = _conn()
|
||||||
if conn is not None:
|
if cache and conn is not None:
|
||||||
try:
|
try:
|
||||||
conn.execute(
|
conn.execute(
|
||||||
"INSERT OR REPLACE INTO derived VALUES (?,?,?,?,?,?)",
|
"INSERT OR REPLACE INTO derived VALUES (?,?,?,?,?,?)",
|
||||||
|
|
@ -115,7 +120,7 @@ def put_payload(kind: str, cell_id: str, key: str, token: str, payload: dict) ->
|
||||||
|
|
||||||
# --- reverse-geocode labels --------------------------------------------------
|
# --- reverse-geocode labels --------------------------------------------------
|
||||||
|
|
||||||
REVGEO_MISS_TTL = 86400.0 # a "no label" result may be transient — retry after a day
|
REVGEO_MISS_TTL = 3600.0 # a "no label" result may be transient (rate limit) — retry hourly
|
||||||
|
|
||||||
|
|
||||||
def revgeo_key(lat: float, lon: float) -> str:
|
def revgeo_key(lat: float, lon: float) -> str:
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue