fix(api,notebooks): unwedge /health, unlock zotero reads, marimo chart crash, #515 flake
Some checks failed
CI / lint (push) Successful in 35s
CI / notebooks-smoke (push) Successful in 1m34s
Deploy / notebooks (push) Has been skipped
Deploy / zotero (push) Has been skipped
Deploy / docs (push) Has been skipped
Deploy / api (push) Successful in 54s
Deploy / mc (push) Has been skipped
Infra CI / notebooks (push) Successful in 58s
Infra CI / zotero (push) Successful in 14s
Infra CI / docs (push) Successful in 1m38s
Infra CI / api (push) Successful in 13s
Infra CI / mc (push) Successful in 14s
Deploy / report (push) Successful in 12s
CI / test (push) Has been cancelled
Some checks failed
CI / lint (push) Successful in 35s
CI / notebooks-smoke (push) Successful in 1m34s
Deploy / notebooks (push) Has been skipped
Deploy / zotero (push) Has been skipped
Deploy / docs (push) Has been skipped
Deploy / api (push) Successful in 54s
Deploy / mc (push) Has been skipped
Infra CI / notebooks (push) Successful in 58s
Infra CI / zotero (push) Successful in 14s
Infra CI / docs (push) Successful in 1m38s
Infra CI / api (push) Successful in 13s
Infra CI / mc (push) Successful in 14s
Deploy / report (push) Successful in 12s
CI / test (push) Has been cancelled
The api container had been unhealthy for ~12h with ~1500 leaked healthcheck zombies. Root cause: /health -> _check_bib -> list_items()[:1] hydrates the ENTIRE bib store via one get() per row (~140s at the 181k items the zotero sync reached) and never closes the connection — every hit pinned a threadpool thread until the pool (40) was exhausted and the event loop had nothing left to respond with. - bib.Store.list_items: SQL-level limit= param; count() is now a single COUNT query (was len(list_items()) — O(n) get() calls); shared _filter_clause builder - api _check_bib: bounded probe (limit=1) + explicit close; /bib/items pushes its limit into SQL instead of slicing after a full scan - api.Dockerfile healthcheck: curl --max-time 4 — docker's timeout only stops waiting; the probe process previously lived on forever - conf.connect.zotero(): mode=ro&immutable=1 — the running Zotero app holds the db lock nearly permanently, so plain ro opens fail with 'database is locked' (nb issue #557) - notebooks: unwrap mo.ui.altair_chart in skin_subs/acodb explorers — marimo 0.23.13 _get_binned_fields crashes on list-valued tooltip encodings ('list' object has no attribute 'get', nb issue #548) - ci test job: preinstall duckdb sqlite extension before pytest — xdist workers raced INSTALL in ~/.duckdb (#515, 3 flaked runs)
This commit is contained in:
@@ -46,6 +46,13 @@ jobs:
|
||||
- name: Install dependencies
|
||||
run: uv sync --dev
|
||||
|
||||
- name: Preinstall duckdb extensions
|
||||
# tests/zot/test_duck.py runs INSTALL sqlite from every xdist
|
||||
# worker; concurrent installs race the extension-file rename in
|
||||
# ~/.duckdb ("Could not remove file ... sqlite_scanner", #515).
|
||||
# Installing once up front makes the in-test INSTALL a no-op.
|
||||
run: uv run python -c "import duckdb; duckdb.connect().execute('INSTALL sqlite')"
|
||||
|
||||
- name: Pytest
|
||||
# -n auto parallelizes across runner cores. Coverage combining
|
||||
# is configured via [tool.coverage.run] parallel=true in
|
||||
|
||||
@@ -177,6 +177,13 @@ jobs:
|
||||
- name: Install dependencies
|
||||
run: uv sync --dev
|
||||
|
||||
- name: Preinstall duckdb extensions
|
||||
# tests/zot/test_duck.py runs INSTALL sqlite from every xdist
|
||||
# worker; concurrent installs race the extension-file rename in
|
||||
# ~/.duckdb ("Could not remove file ... sqlite_scanner", #515).
|
||||
# Installing once up front makes the in-test INSTALL a no-op.
|
||||
run: uv run python -c "import duckdb; duckdb.connect().execute('INSTALL sqlite')"
|
||||
|
||||
- name: Pytest
|
||||
# -n auto parallelizes across runner cores. Coverage combining
|
||||
# is configured via [tool.coverage.run] parallel=true in
|
||||
|
||||
@@ -28,8 +28,11 @@ COPY stack.toml ./
|
||||
|
||||
EXPOSE 8000
|
||||
|
||||
# --max-time bounds curl itself: docker's timeout only stops *waiting* —
|
||||
# the probe process lives on, and a wedged server once accumulated ~1500
|
||||
# hung healthcheck zombies this way.
|
||||
HEALTHCHECK --interval=30s --timeout=5s --retries=3 \
|
||||
CMD curl -sf http://localhost:8000/health || exit 1
|
||||
CMD curl -sf --max-time 4 http://localhost:8000/health || exit 1
|
||||
|
||||
CMD ["uv", "run", "--no-sync", "uvicorn", "api.server:app", \
|
||||
"--host", "0.0.0.0", "--port", "8000", \
|
||||
|
||||
@@ -193,7 +193,7 @@ def _(PALETTE, alt, mo, q):
|
||||
.properties(title="Encounters by Type", width=600, height=450)
|
||||
)
|
||||
|
||||
mo.ui.altair_chart(enc_chart)
|
||||
enc_chart
|
||||
return
|
||||
|
||||
|
||||
@@ -226,7 +226,7 @@ def _(PALETTE, alt, mo, q):
|
||||
.properties(title="Monthly Encounter Trends (Key Types)", width=800, height=350)
|
||||
)
|
||||
|
||||
mo.ui.altair_chart(trend_chart)
|
||||
trend_chart
|
||||
return
|
||||
|
||||
|
||||
@@ -266,7 +266,7 @@ def _(PALETTE, alt, mo, q):
|
||||
)
|
||||
)
|
||||
|
||||
mo.ui.altair_chart(chronic_chart)
|
||||
chronic_chart
|
||||
return
|
||||
|
||||
|
||||
@@ -618,7 +618,7 @@ def _(PALETTE, alt, mo, q):
|
||||
.properties(title="Top 15 ED Diagnoses", width=700, height=400)
|
||||
)
|
||||
|
||||
mo.ui.altair_chart(ed_dx_chart)
|
||||
ed_dx_chart
|
||||
return
|
||||
|
||||
|
||||
@@ -690,7 +690,7 @@ def _(PALETTE, alt, mo, q):
|
||||
.properties(title="Readmission Rate by Specialty Cohort", width=600, height=250)
|
||||
)
|
||||
|
||||
mo.ui.altair_chart(cohort_chart)
|
||||
cohort_chart
|
||||
return
|
||||
|
||||
|
||||
@@ -809,7 +809,7 @@ def _(PALETTE, alt, mo, q):
|
||||
.properties(title="PQI Observed Rates", width=700, height=300)
|
||||
)
|
||||
|
||||
mo.ui.altair_chart(pqi_chart)
|
||||
pqi_chart
|
||||
return
|
||||
|
||||
|
||||
|
||||
@@ -122,7 +122,7 @@ def _(alt, asp, mo, pl, product_select):
|
||||
.encode(y=alt.datum(127.28), text=alt.datum("$127.28 flat rate (Jan 2026)"))
|
||||
)
|
||||
|
||||
mo.ui.altair_chart(chart + flat_rate_line + flat_label)
|
||||
chart + flat_rate_line + flat_label
|
||||
return
|
||||
|
||||
|
||||
@@ -185,7 +185,7 @@ def _(alt, con, mo, pl):
|
||||
.properties(width=600, height=300, title="Products by Category")
|
||||
)
|
||||
|
||||
mo.vstack([mo.ui.altair_chart(mfr_chart), mo.ui.altair_chart(cat_chart)])
|
||||
mo.vstack([mfr_chart, cat_chart])
|
||||
return
|
||||
|
||||
|
||||
@@ -251,7 +251,7 @@ def _(alt, con, mo, pl):
|
||||
.properties(width=600, height=250, title="Total Paid by MAC Jurisdiction")
|
||||
)
|
||||
|
||||
mo.vstack([mo.ui.altair_chart(geo_chart), mo.ui.altair_chart(mac_chart)])
|
||||
mo.vstack([geo_chart, mac_chart])
|
||||
return
|
||||
|
||||
|
||||
@@ -279,7 +279,7 @@ def _(alt, con, mo, pl):
|
||||
.properties(width=600, height=300, title="Spend by Specialty and Setting")
|
||||
)
|
||||
|
||||
mo.ui.altair_chart(setting_chart)
|
||||
setting_chart
|
||||
return
|
||||
|
||||
|
||||
@@ -336,7 +336,7 @@ def _(alt, con, mo, pl):
|
||||
tier_table = mo.ui.table(tier.to_pandas())
|
||||
|
||||
mo.vstack(
|
||||
[mo.ui.altair_chart(scatter), mo.md("### Risk Tier Distribution"), tier_table]
|
||||
[scatter, mo.md("### Risk Tier Distribution"), tier_table]
|
||||
)
|
||||
return
|
||||
|
||||
|
||||
@@ -35,7 +35,9 @@ def list_items(
|
||||
kwargs["tag"] = tag
|
||||
if item_type:
|
||||
kwargs["item_type"] = item_type
|
||||
items = store.list_items(**kwargs)[:limit]
|
||||
# SQL-level limit: hydrating rows costs a get() each, so slicing
|
||||
# after the fact scans the whole store.
|
||||
items = store.list_items(**kwargs, limit=limit)
|
||||
return [
|
||||
BibItem(key=i.key or "", title=i.title, item_type=type(i).__name__)
|
||||
for i in items
|
||||
|
||||
@@ -50,8 +50,16 @@ def _check_bib() -> ServiceCheck:
|
||||
|
||||
from bib.store import Store
|
||||
|
||||
# Bounded probe + explicit close: this runs on every /health hit
|
||||
# from a threadpool thread. The previous list_items()[:1] hydrated
|
||||
# the whole store (N+1 get() per item — minutes at 180k items) and
|
||||
# leaked the connection, which exhausted the pool and wedged the
|
||||
# entire API once the Zotero sync grew the database.
|
||||
store = Store(str(bib_path))
|
||||
count = len(store.list_items()[:1])
|
||||
try:
|
||||
count = len(store.list_items(limit=1))
|
||||
finally:
|
||||
store.close()
|
||||
return ServiceCheck(name="bib", status="ok", detail=f"{count}+ items")
|
||||
except Exception as e:
|
||||
return ServiceCheck(name="bib", status="degraded", detail=str(e))
|
||||
|
||||
@@ -240,17 +240,10 @@ class Store:
|
||||
|
||||
# ── Query ────────────────────────────────────────────────────
|
||||
|
||||
def list_items(
|
||||
self,
|
||||
*,
|
||||
tag: str = "",
|
||||
item_type: str = "",
|
||||
collection: str = "",
|
||||
query: str = "",
|
||||
) -> list[Item]:
|
||||
"""List items with optional filters."""
|
||||
con = self._con()
|
||||
sql = "SELECT DISTINCT i.key FROM items i"
|
||||
def _filter_clause(
|
||||
self, tag: str, item_type: str, collection: str, query: str
|
||||
) -> tuple[str, list[Any]]:
|
||||
"""Build the shared JOIN/WHERE suffix for item filters."""
|
||||
joins: list[str] = []
|
||||
wheres: list[str] = []
|
||||
params: list[Any] = []
|
||||
@@ -279,17 +272,50 @@ class Store:
|
||||
wheres.append("(i.title LIKE ? OR i.abstract LIKE ?)")
|
||||
params.extend([f"%{query}%", f"%{query}%"])
|
||||
|
||||
full_sql = sql + "".join(joins)
|
||||
clause = "".join(joins)
|
||||
if wheres:
|
||||
full_sql += " WHERE " + " AND ".join(wheres)
|
||||
full_sql += " ORDER BY i.id"
|
||||
clause += " WHERE " + " AND ".join(wheres)
|
||||
return clause, params
|
||||
|
||||
def list_items(
|
||||
self,
|
||||
*,
|
||||
tag: str = "",
|
||||
item_type: str = "",
|
||||
collection: str = "",
|
||||
query: str = "",
|
||||
limit: int | None = None,
|
||||
) -> list[Item]:
|
||||
"""List items with optional filters.
|
||||
|
||||
Pass ``limit`` to bound the scan — each returned item costs a
|
||||
``get()`` round-trip, so an unbounded call on a large store
|
||||
(Zotero sync is ~180k items) takes minutes.
|
||||
"""
|
||||
con = self._con()
|
||||
clause, params = self._filter_clause(tag, item_type, collection, query)
|
||||
full_sql = f"SELECT DISTINCT i.key FROM items i{clause} ORDER BY i.id"
|
||||
if limit is not None:
|
||||
full_sql += " LIMIT ?"
|
||||
params.append(limit)
|
||||
|
||||
rows = con.execute(full_sql, params).fetchall()
|
||||
return [self.get(r["key"]) for r in rows]
|
||||
|
||||
def count(self, **filters: Any) -> int:
|
||||
"""Count items matching filters."""
|
||||
return len(self.list_items(**filters))
|
||||
def count(
|
||||
self,
|
||||
*,
|
||||
tag: str = "",
|
||||
item_type: str = "",
|
||||
collection: str = "",
|
||||
query: str = "",
|
||||
) -> int:
|
||||
"""Count items matching filters (single COUNT query — no item
|
||||
hydration; ``len(list_items())`` is O(n) get() calls)."""
|
||||
con = self._con()
|
||||
clause, params = self._filter_clause(tag, item_type, collection, query)
|
||||
full_sql = f"SELECT COUNT(DISTINCT i.key) AS n FROM items i{clause}"
|
||||
return int(con.execute(full_sql, params).fetchone()["n"])
|
||||
|
||||
def to_dataframe(self, **filters: Any) -> Any:
|
||||
"""Export items as a polars DataFrame (lazy import)."""
|
||||
|
||||
@@ -77,10 +77,19 @@ def bib() -> Any:
|
||||
return Store(str(path("db.bib")))
|
||||
|
||||
|
||||
def zotero() -> sqlite3.Connection:
|
||||
"""Return a read-only SQLite connection to the Zotero database."""
|
||||
def zotero(*, immutable: bool = True) -> sqlite3.Connection:
|
||||
"""Return a read-only SQLite connection to the Zotero database.
|
||||
|
||||
``immutable=1`` bypasses SQLite's locking protocol entirely — required
|
||||
because the running Zotero app holds the database lock almost
|
||||
permanently, so a plain ``mode=ro`` connection fails with "database is
|
||||
locked". The trade-off: a read that races an actual Zotero write can
|
||||
see a torn snapshot. Pass ``immutable=False`` if you need a consistent
|
||||
read and can guarantee Zotero is stopped.
|
||||
"""
|
||||
db = path("db.zotero")
|
||||
return sqlite3.connect(f"file:{db}?mode=ro", uri=True)
|
||||
uri = f"file:{db}?mode=ro" + ("&immutable=1" if immutable else "")
|
||||
return sqlite3.connect(uri, uri=True)
|
||||
|
||||
|
||||
def nessie(*, base_url: str = "") -> Any:
|
||||
|
||||
@@ -360,6 +360,21 @@ class TestListItems:
|
||||
assert populated_store.count(tag="module:pfs") == 3
|
||||
populated_store.close()
|
||||
|
||||
def test_count_all_filters(self, populated_store: Store) -> None:
|
||||
assert populated_store.count(item_type="rule") == 1
|
||||
assert populated_store.count(query="RVU") == 1
|
||||
assert populated_store.count(tag="nonexistent") == 0
|
||||
populated_store.close()
|
||||
|
||||
def test_list_limit(self, populated_store: Store) -> None:
|
||||
items = populated_store.list_items(limit=2)
|
||||
assert len(items) == 2
|
||||
# limit composes with filters
|
||||
assert len(populated_store.list_items(tag="module:pfs", limit=1)) == 1
|
||||
# limit larger than result set is a no-op
|
||||
assert len(populated_store.list_items(limit=100)) == 4
|
||||
populated_store.close()
|
||||
|
||||
def test_to_dataframe(self, populated_store: Store) -> None:
|
||||
import polars as pl
|
||||
|
||||
|
||||
Reference in New Issue
Block a user