Automate checklist-backend updates
This commit is contained in:
14
.env.example
14
.env.example
@@ -213,11 +213,17 @@ UPLOAD_AUTORUN=true
|
||||
# exists to avoid. Stage 6 is the slowest stage and reaches the network, but
|
||||
# only one batch runs at a time, so nothing else competes with it.
|
||||
#
|
||||
# LLM off, because use_llm gates only description generation in stage 2, and
|
||||
# production runs USE_OLLAMA=false - turning it on there buys nothing and costs
|
||||
# a connection timeout per row.
|
||||
# LLM on, though in production it currently does nothing: USE_OLLAMA is false
|
||||
# there, so the LLM call returns immediately without a request and the row keeps
|
||||
# its blank description. Set true so the pipeline is already right for the day
|
||||
# an Ollama server is reachable.
|
||||
#
|
||||
# If you DO set USE_OLLAMA=true, make sure something is actually listening on
|
||||
# OLLAMA_BASE_URL. An unreachable server costs a 5s probe, and while that probe
|
||||
# is cached per 30s rather than paid per row, a reachable-but-slow model is
|
||||
# billed per row at OLLAMA_TIMEOUT_SECONDS.
|
||||
UPLOAD_AUTORUN_FETCH_IMAGES=true
|
||||
UPLOAD_AUTORUN_USE_LLM=false
|
||||
UPLOAD_AUTORUN_USE_LLM=true
|
||||
|
||||
USE_OLLAMA=true
|
||||
OLLAMA_BASE_URL=http://localhost:11434
|
||||
|
||||
@@ -190,11 +190,21 @@ UPLOAD_AUTORUN = _bool("UPLOAD_AUTORUN", "true")
|
||||
# exists to avoid - stage 6 is the slowest stage and reaches the network, but
|
||||
# only one batch runs at a time so nothing else is competing with it.
|
||||
#
|
||||
# LLM OFF, because `use_llm` gates only description generation in
|
||||
# stage_2_row_intake, and production runs USE_OLLAMA=false: turning it on there
|
||||
# buys nothing and costs a connection timeout per row.
|
||||
# LLM ON. `use_llm` gates only description generation in stage_2_row_intake, and
|
||||
# in production it is currently a no-op: USE_OLLAMA is false there, so
|
||||
# ollama_service._ensure_client() returns on its first line without a request
|
||||
# and the row simply keeps its blank description. It is set true so the pipeline
|
||||
# is already configured correctly for the day an Ollama server exists.
|
||||
#
|
||||
# This default USED to be false, on the grounds that turning it on "costs a
|
||||
# connection timeout per row". That was true, and it was about the OTHER branch
|
||||
# of _ensure_client - USE_OLLAMA=true with nothing listening, which is any
|
||||
# developer machine that has not run `ollama serve`. It is answered now by the
|
||||
# TTL cache on that probe rather than by leaving the feature off: one probe per
|
||||
# batch instead of one per row. Do not remove that cache and this default
|
||||
# together without re-reading why both exist.
|
||||
UPLOAD_AUTORUN_FETCH_IMAGES = _bool("UPLOAD_AUTORUN_FETCH_IMAGES", "true")
|
||||
UPLOAD_AUTORUN_USE_LLM = _bool("UPLOAD_AUTORUN_USE_LLM", "false")
|
||||
UPLOAD_AUTORUN_USE_LLM = _bool("UPLOAD_AUTORUN_USE_LLM", "true")
|
||||
|
||||
# --- Review inbox ----------------------------------------------------------
|
||||
# The bound that applies only when UPLOAD_AUTORUN is false. Files then wait in
|
||||
|
||||
@@ -3,6 +3,8 @@ from __future__ import annotations
|
||||
from typing import List, Dict, Any, Optional
|
||||
import json
|
||||
import re
|
||||
import time
|
||||
|
||||
import requests
|
||||
|
||||
from app.infrastructure.settings import OLLAMA_BASE_URL, OLLAMA_MODEL_NAME, USE_OLLAMA, OLLAMA_TIMEOUT_SECONDS
|
||||
@@ -20,15 +22,58 @@ SYSTEM_PROMPT = (
|
||||
)
|
||||
|
||||
|
||||
# Reachability is asked once per this many seconds, not once per caller.
|
||||
#
|
||||
# WHY THIS CACHE EXISTS. The probe below costs up to 5 seconds when nothing is
|
||||
# listening, and `_ensure_client` is called per ROW by stage 2 of the ingestion
|
||||
# pipeline (store_catalog_pipeline.stage_2_row_intake -> fetch_product_details).
|
||||
# Uncached, a 2000-row sheet ingested with use_llm on, against a configured but
|
||||
# unreachable Ollama, spends up to ~2.8 hours doing nothing but timing out - and
|
||||
# presents as a batch that has hung rather than one that has failed. That is not
|
||||
# hypothetical: USE_OLLAMA=true pointing at localhost:11434 is the default
|
||||
# developer configuration, and `ollama serve` is not always running beside it.
|
||||
#
|
||||
# /api/health calls this too (app/api/routers/system.py), so the same cache
|
||||
# stops a down Ollama adding 5s to every health request.
|
||||
#
|
||||
# A TTL rather than a permanent memo, deliberately: this is a liveness fact, not
|
||||
# configuration. Cached forever, an Ollama started after the API would never be
|
||||
# noticed and /api/health would report it down until a redeploy.
|
||||
_PROBE_TTL_SECONDS = 30.0
|
||||
_probe_cache: tuple[float, bool] | None = None
|
||||
|
||||
|
||||
def reset_reachability_cache() -> None:
|
||||
"""Forget the cached probe. For tests, and for anything that knows the
|
||||
answer just changed."""
|
||||
global _probe_cache
|
||||
_probe_cache = None
|
||||
|
||||
|
||||
def _ensure_client():
|
||||
"""None when Ollama is switched off, True/False for reachable or not.
|
||||
|
||||
Three return values, not two - `system.py` relies on telling "disabled" from
|
||||
"configured but down", so do not collapse this to a bool.
|
||||
"""
|
||||
if not USE_OLLAMA:
|
||||
# No network call on this path, so nothing worth caching.
|
||||
return None
|
||||
|
||||
global _probe_cache
|
||||
now = time.monotonic()
|
||||
if _probe_cache is not None and now - _probe_cache[0] < _PROBE_TTL_SECONDS:
|
||||
return _probe_cache[1]
|
||||
|
||||
# Verify Ollama is reachable
|
||||
try:
|
||||
resp = requests.get(f"{OLLAMA_BASE_URL}/api/tags", timeout=5)
|
||||
return resp.status_code == 200
|
||||
reachable = resp.status_code == 200
|
||||
except Exception:
|
||||
return False
|
||||
reachable = False
|
||||
|
||||
_probe_cache = (now, reachable)
|
||||
return reachable
|
||||
|
||||
|
||||
def _generate(system: str, user_prompt: str, max_retries: int = 2) -> str:
|
||||
|
||||
@@ -116,9 +116,14 @@ curl -X POST https://mcp.nearle.ai.in/api/uploads/catalog \
|
||||
| `files` | **required** | The spreadsheets. Repeat the field for more than one; up to 20. |
|
||||
| `sender` | optional | A label for the inbox, so the admin can see who sent what. Free text, trimmed to 60 chars. Defaults to `anonymous`. |
|
||||
|
||||
`use_llm` and `fetch_images` are **no longer accepted here.** They decide how a run
|
||||
behaves and commit the host to outbound work, so the choice belongs to the admin
|
||||
pressing Start — not to the sender. Passing them is inert.
|
||||
`use_llm` and `fetch_images` are **not accepted here.** They commit the host to
|
||||
outbound work, and this endpoint's caller is anonymous, so the choice is not theirs to
|
||||
make. Passing them is inert — the response reports what was actually used.
|
||||
|
||||
Both are **on** for an auto-started run: image search fills the product images, and the
|
||||
LLM fills blank descriptions wherever an Ollama server is reachable. There is none in
|
||||
production today, so in practice descriptions arrive exactly as your sheet wrote them
|
||||
and the run is otherwise unaffected.
|
||||
|
||||
### Which columns are read
|
||||
|
||||
@@ -163,7 +168,7 @@ The bad file is kept as a failed member rather than dropped, so a sender who sub
|
||||
"files_done": 0,
|
||||
"files_failed": 1,
|
||||
"current_file": null,
|
||||
"use_llm": false, // UPLOAD_AUTORUN_USE_LLM
|
||||
"use_llm": true, // UPLOAD_AUTORUN_USE_LLM
|
||||
"fetch_images": true, // UPLOAD_AUTORUN_FETCH_IMAGES
|
||||
"runner": "inprocess",
|
||||
"totals": { "rows_total": 0, "products_built": 0, "inserted": 0,
|
||||
|
||||
121
tests/test_ollama_reachability.py
Normal file
121
tests/test_ollama_reachability.py
Normal file
@@ -0,0 +1,121 @@
|
||||
"""The reachability probe in front of every Ollama call.
|
||||
|
||||
WHY THIS FILE EXISTS
|
||||
--------------------
|
||||
`_ensure_client()` asks Ollama for `/api/tags` with a 5-second timeout, and
|
||||
`stage_2_row_intake` calls it once per ROW through `fetch_product_details`.
|
||||
Uncached, a 2000-row sheet ingested with `use_llm` on, against a configured but
|
||||
unreachable Ollama, spends up to ~2.8 hours doing nothing but timing out - and
|
||||
shows as a batch that has hung, not one that has failed.
|
||||
|
||||
That was survivable only while `use_llm` defaulted to false everywhere. It no
|
||||
longer does: `UPLOAD_AUTORUN_USE_LLM` is true, so every auto-started upload now
|
||||
takes this path. The cache is what makes that default safe, and the first test
|
||||
below is the one that stops it being quietly removed in a later refactor.
|
||||
|
||||
`/api/health` calls the same function, so a down Ollama also stops adding five
|
||||
seconds to every health request.
|
||||
"""
|
||||
from __future__ import annotations
|
||||
|
||||
import pytest
|
||||
|
||||
from app.services import ollama_service
|
||||
|
||||
|
||||
@pytest.fixture(autouse=True)
|
||||
def _clean_probe_cache():
|
||||
"""The cache is a module global and outlives a test."""
|
||||
ollama_service.reset_reachability_cache()
|
||||
yield
|
||||
ollama_service.reset_reachability_cache()
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def probe_calls(monkeypatch):
|
||||
"""Count the HTTP probes, and make every one of them fail.
|
||||
|
||||
Failure is the case that matters: a reachable Ollama answers in
|
||||
milliseconds, an unreachable one costs the full timeout, and it is the
|
||||
second that used to be paid per row.
|
||||
"""
|
||||
calls: list = []
|
||||
|
||||
def boom(url, **kwargs):
|
||||
calls.append(url)
|
||||
raise OSError("connection refused")
|
||||
|
||||
monkeypatch.setattr(ollama_service, "USE_OLLAMA", True)
|
||||
monkeypatch.setattr(ollama_service.requests, "get", boom)
|
||||
return calls
|
||||
|
||||
|
||||
def test_an_unreachable_ollama_is_probed_once_not_once_per_call(probe_calls):
|
||||
"""The whole point. Ten rows must not be ten timeouts."""
|
||||
for _ in range(10):
|
||||
assert ollama_service._ensure_client() is False
|
||||
|
||||
assert len(probe_calls) == 1, (
|
||||
f"{len(probe_calls)} probes for 10 calls - the cache is not holding, and "
|
||||
f"an ingest will pay the 5s timeout per row"
|
||||
)
|
||||
|
||||
|
||||
def test_the_cache_expires_so_a_late_start_is_noticed(probe_calls, monkeypatch):
|
||||
"""A permanent memo would mean an Ollama started after the API is never
|
||||
seen, and /api/health reports it down until someone redeploys."""
|
||||
clock = [1000.0]
|
||||
monkeypatch.setattr(ollama_service.time, "monotonic", lambda: clock[0])
|
||||
|
||||
ollama_service._ensure_client()
|
||||
assert len(probe_calls) == 1
|
||||
|
||||
clock[0] += ollama_service._PROBE_TTL_SECONDS + 1
|
||||
ollama_service._ensure_client()
|
||||
assert len(probe_calls) == 2, "the probe never expired"
|
||||
|
||||
|
||||
def test_a_reachable_ollama_is_also_cached(monkeypatch):
|
||||
"""Both outcomes are cached. Caching only the failure would leave the happy
|
||||
path paying an HTTP round trip per row - cheap, but per row and pointless."""
|
||||
calls: list = []
|
||||
|
||||
class Ok:
|
||||
status_code = 200
|
||||
|
||||
def ok(url, **kwargs):
|
||||
calls.append(url)
|
||||
return Ok()
|
||||
|
||||
monkeypatch.setattr(ollama_service, "USE_OLLAMA", True)
|
||||
monkeypatch.setattr(ollama_service.requests, "get", ok)
|
||||
|
||||
assert [ollama_service._ensure_client() for _ in range(5)] == [True] * 5
|
||||
assert len(calls) == 1
|
||||
|
||||
|
||||
def test_disabled_stays_none_and_never_touches_the_network(monkeypatch):
|
||||
"""Three return values, not two: `system.py` tells "switched off" from
|
||||
"configured but down", and /api/health's `ollama` field means different
|
||||
things in each case. Collapsing this to a bool would break that.
|
||||
"""
|
||||
def never(*_args, **_kwargs):
|
||||
raise AssertionError("USE_OLLAMA is false - nothing may be requested")
|
||||
|
||||
monkeypatch.setattr(ollama_service, "USE_OLLAMA", False)
|
||||
monkeypatch.setattr(ollama_service.requests, "get", never)
|
||||
|
||||
assert ollama_service._ensure_client() is None
|
||||
|
||||
|
||||
def test_a_row_keeps_its_description_when_ollama_is_absent(probe_calls):
|
||||
"""The pipeline's side of the contract. `use_llm` being on must be a no-op
|
||||
against an unreachable server, not a failure - stage 2 is best-effort and
|
||||
every later stage has to keep working."""
|
||||
from app.core.store_catalog_pipeline import stage_2_row_intake
|
||||
|
||||
row = stage_2_row_intake(
|
||||
{"product_name": "Amul Butter 100g", "brand": "Amul", "description": ""},
|
||||
use_llm=True,
|
||||
)
|
||||
assert row["description"] == ""
|
||||
@@ -127,31 +127,44 @@ def test_the_run_is_attributed_to_the_sender_label(client):
|
||||
# ---------------------------------------------------------------------------
|
||||
# What the anonymous caller still cannot do
|
||||
# ---------------------------------------------------------------------------
|
||||
def test_the_sender_cannot_switch_on_the_expensive_stages(client):
|
||||
def test_the_request_cannot_move_either_stage(client):
|
||||
"""Image search and the LLM reach the network and are chosen by settings,
|
||||
never by the request. An anonymous caller who could flip these could commit
|
||||
a one-vCPU host to outbound work at will."""
|
||||
a one-vCPU host to outbound work at will.
|
||||
|
||||
Both fields are sent OPPOSITE to their configured values, which is the only
|
||||
way this proves anything: if the endpoint started honouring the request, the
|
||||
response would have to disagree with the settings, and it must not.
|
||||
"""
|
||||
body = client.post(
|
||||
UPLOAD,
|
||||
files=_files(("a.csv", _csv())),
|
||||
data={"use_llm": "true", "fetch_images": "false"},
|
||||
data={"use_llm": "false", "fetch_images": "false"},
|
||||
).json()
|
||||
|
||||
# The configured values, not the ones asked for.
|
||||
assert body["use_llm"] is False
|
||||
assert body["use_llm"] is True
|
||||
assert body["fetch_images"] is True
|
||||
|
||||
|
||||
def test_both_stages_are_on_for_an_auto_started_run(client):
|
||||
"""The requirement, pinned against the shipped defaults rather than against
|
||||
a monkeypatched pair - so flipping either default in settings.py fails here
|
||||
rather than passing quietly."""
|
||||
body = client.post(UPLOAD, files=_files(("a.csv", _csv()))).json()
|
||||
assert body["fetch_images"] is True
|
||||
assert body["use_llm"] is True
|
||||
|
||||
|
||||
def test_the_configured_defaults_are_what_reach_the_run(client, monkeypatch):
|
||||
"""Both directions, so a wrong default cannot hide behind a matching one."""
|
||||
from app.api.routers import uploads
|
||||
|
||||
monkeypatch.setattr(uploads, "UPLOAD_AUTORUN_FETCH_IMAGES", False)
|
||||
monkeypatch.setattr(uploads, "UPLOAD_AUTORUN_USE_LLM", True)
|
||||
monkeypatch.setattr(uploads, "UPLOAD_AUTORUN_USE_LLM", False)
|
||||
|
||||
body = client.post(UPLOAD, files=_files(("a.csv", _csv()))).json()
|
||||
assert body["fetch_images"] is False
|
||||
assert body["use_llm"] is True
|
||||
assert body["use_llm"] is False
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
Reference in New Issue
Block a user