From 753f9b2c2adae6ea0e98a5aece0731cca93e99e4 Mon Sep 17 00:00:00 2001 From: Alex Garcia Date: Thu, 30 Jul 2026 17:42:41 -0700 Subject: [PATCH] Ensure immutable table counts still precompute when startup ran first Co-Authored-By: Claude Fable 5 --- datasette/app.py | 14 +++++++------ datasette/cli.py | 7 +++++-- tests/test_lifespan.py | 47 ++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 60 insertions(+), 8 deletions(-) diff --git a/datasette/app.py b/datasette/app.py index 29ea7ae3..42be7425 100644 --- a/datasette/app.py +++ b/datasette/app.py @@ -2815,15 +2815,17 @@ class Datasette: This is the single entry point used by both AsgiLifespan (so real deployments finish startup before accepting requests) and AsgiRunOnFirstRequest (the fallback for hosts that never send - lifespan events, e.g. DatasetteClient's httpx.ASGITransport). It - may also race an explicit `await ds.invoke_startup()` call made by - `datasette serve` before the server starts serving - that's fine, - `invoke_startup()` has its own `_startup_invoked` guard. + lifespan events, e.g. DatasetteClient's httpx.ASGITransport), and + `datasette serve` (cli.py) calls it too. The fast path below checks + both `_startup_invoked` and `_setup_db_done` - not just the former - + so that a bare `await ds.invoke_startup()` made by a caller ahead of + `_startup_sequence()` (which only sets `_startup_invoked`) can't + make this method skip the immutable-database table-count precompute. """ - if self._startup_invoked: + if self._startup_invoked and self._setup_db_done: return async with self._startup_lock: - if self._startup_invoked: + if self._startup_invoked and self._setup_db_done: return if not self._setup_db_done: # First time server starts up, calculate table counts for diff --git a/datasette/cli.py b/datasette/cli.py index 12024a14..2694c1f6 100644 --- a/datasette/cli.py +++ b/datasette/cli.py @@ -710,9 +710,12 @@ def serve( # Populate internal catalog tables before invoke_startup await check_databases(ds) - # Run the "startup" plugin hooks + # Run the full startup sequence (immutable-database table-count + # precompute + the "startup" plugin hooks) via the same entry point + # AsgiLifespan/AsgiRunOnFirstRequest use, so it's not skipped when + # uvicorn's lifespan.startup fires moments later. try: - await ds.invoke_startup() + await ds._startup_sequence() except StartupError as e: raise click.ClickException(e.args[0]) diff --git a/tests/test_lifespan.py b/tests/test_lifespan.py index 5efda0e6..7de8ec42 100644 --- a/tests/test_lifespan.py +++ b/tests/test_lifespan.py @@ -13,9 +13,11 @@ import asyncio import contextlib import httpx import pytest +import sqlite3 from datasette import hookimpl from datasette.app import Datasette +from datasette.database import Database from datasette.plugins import pm @@ -209,3 +211,48 @@ async def test_concurrent_first_requests_all_wait_for_slow_startup(): assert all(response.status_code == 200 for response in responses) assert call_count["n"] == 1 assert ds._startup_invoked is True + + +@pytest.mark.asyncio +async def test_setup_db_still_runs_when_invoke_startup_ran_first(tmp_path, monkeypatch): + # Regression test: `datasette serve` (cli.py _serve_async) calls + # ds.invoke_startup() directly, before uvicorn ever sends a + # lifespan.startup event that drives _startup_sequence(). If + # _startup_sequence()'s fast path only checked `_startup_invoked`, it + # would see startup already done and skip the immutable-database + # table-count precompute (setup_db) entirely - a silent regression + # versus main, where AsgiRunOnFirstRequest ran setup_db unconditionally + # on request #1. + db_path = tmp_path / "immutable.db" + conn = sqlite3.connect(str(db_path)) + conn.execute("create table t (id integer primary key)") + conn.commit() + conn.close() + + ds = Datasette([], immutables=[str(db_path)]) + + call_count = {"n": 0} + original_table_counts = Database.table_counts + + async def counting_table_counts(self, *args, **kwargs): + call_count["n"] += 1 + return await original_table_counts(self, *args, **kwargs) + + monkeypatch.setattr(Database, "table_counts", counting_table_counts) + + # Simulate the CLI path: invoke_startup() runs directly and completes + # BEFORE _startup_sequence() ever gets a chance to run setup_db. + await ds.invoke_startup() + assert ds._startup_invoked is True + assert call_count["n"] == 0 + + # The lifespan/first-request path (or the CLI itself, per the fix) + # calling the shared entry point afterwards must still precompute + # table counts for immutable databases. + await ds._startup_sequence() + assert call_count["n"] == 1 + assert ds._setup_db_done is True + + # Idempotency: a second call must not recompute. + await ds._startup_sequence() + assert call_count["n"] == 1