ddebc7b
commit ddebc7b
6 files changed
+192 -59
Showing 6 changed files+192-59
▼BENCHMARKS.md
+9-0
| @@ -1453,3 +1453,12 @@ against live Aiven Postgres now also covers: | ||
1453 | 1453 | All endpoints return 200 ("DBPOOL SMOKE OK"). Same ~110 ms/round-trip Aiven |
1454 | 1454 | network bound as before — no added latency. CLI `justapi create <app> --db |
1455 | 1455 | {postgres,mysql,sqlite}` scaffolds a DB-wired project for each engine (verified). |
1456 | + | |
1457 | +### P1 follow-up 2 — `app.db` usable before `run()` (2026-07-17) | |
1458 | + | |
1459 | +`JustAPIApp.connect_database()` resolves the pool eagerly (kept alive on a | |
1460 | +dedicated runtime); the `app.db` property lazy-connects on first access. Verified | |
1461 | +against Aiven Postgres in `benchmarks/smoke_dbpool_prerun.py`: `app.db` is live | |
1462 | +**before any socket is bound** — `SELECT 1`, `DELETE`/`INSERT`, `SELECT` all | |
1463 | +succeed pre-`run()`, and a served route reuses the same pool. No added latency | |
1464 | +(same ~110 ms Aiven network bound). Gates green. | |
1456 | 1465 | |
▼DECISIONS.md
+33-0
| @@ -2125,3 +2125,36 @@ Python code could not run unmodified across all three engines — defeating the | ||
2125 | 2125 | postgres, mysql, and sqlite. `cargo test --workspace --features db` green; |
2126 | 2126 | `cargo clippy --workspace --tests --features justapi-core/db -D warnings` clean; |
2127 | 2127 | `cargo fmt --check` clean. |
2128 | + | |
2129 | +## ADR-057 — 2026-07-17 — Eager/early DB pool connect removes `app.db is None` footgun | |
2130 | + | |
2131 | +**Context:** `app.db` returned `None` until `app.run()` had started, because the | |
2132 | +pool was only resolved inside the server entrypoint's detached async block. | |
2133 | +Application code that wanted to touch the DB before serving — a migration step, | |
2134 | +a REPL, or a test that exercises `app.db.query(...)` without binding a socket — | |
2135 | +had no way to get a live `DbPool`. This was a recurring footgun. | |
2136 | + | |
2137 | +**Decision / changes:** | |
2138 | +1. **`JustAPIApp::connect_database(py)`** (crates/justapi-py/src/native/app.rs): | |
2139 | + resolves the configured `Database` into an `AnyPool` immediately. Idempotent | |
2140 | + (returns early if `db_pool` already set). The DB round-trip runs with the GIL | |
2141 | + released on a dedicated tokio runtime that is **kept alive on the app** | |
2142 | + (`db_runtime: Option<Runtime>`) so the `DbPool`'s `Handle` stays valid for the | |
2143 | + whole process. Bootstrap `init_sql` and the optional background health-check | |
2144 | + loop are applied here. | |
2145 | +2. **Lazy-by-default on the Python side:** `JustAPIApp.db` property now calls | |
2146 | + `connect_database()` on first access (swallowing only the "not configured" | |
2147 | + case) and returns the resolved `DbPool`. So `app.db` works both before and | |
2148 | + after `run()`. Added an explicit `app.connect_database()` for callers who want | |
2149 | + to surface connect errors eagerly. | |
2150 | +3. **`run()` reuses the same path:** it now resolves the pool via | |
2151 | + `connect_database` before the detached loop (guarded by `db_pool.is_none()`), | |
2152 | + then hands the inner `AnyPool` to the Rust-native CRUD handler via | |
2153 | + `DbPool::as_any_pool()`. No double-connect, no second runtime for the server | |
2154 | + case. | |
2155 | + | |
2156 | +**Evidence:** New `benchmarks/smoke_dbpool_prerun.py` asserts `app.db is not None` | |
2157 | +and runs `SELECT 1`, a `DELETE`/`INSERT`, and a `SELECT` **before any socket is | |
2158 | +bound**; then serves a route that queries the same pool. Against live Aiven | |
2159 | +Postgres: "FOOTGUN FIX VERIFIED". `cargo clippy -p justapi-py --tests -D warnings` | |
2160 | +clean; `cargo test --workspace --features db` green. | |
2128 | 2161 | |
▼benchmarks/smoke_dbpool_prerun.py
+34-0
| @@ -0,0 +1,34 @@ | ||
1 | +import time, threading, urllib.request | |
2 | +from justapi import JustAPIApp, Database | |
3 | + | |
4 | +URL = "postgres://avnadmin:<SECRET_PASSWORD>@dexmorgan-examhallorhell-5f8c.d.aivencloud.com:11100/defaultdb?sslmode=require" | |
5 | + | |
6 | +app = JustAPIApp() | |
7 | +app.set_database( | |
8 | + URL, | |
9 | + init_sql="CREATE TABLE IF NOT EXISTS dbpool_demo (id SERIAL PRIMARY KEY, name TEXT, qty INT)", | |
10 | +) | |
11 | + | |
12 | +# FOOTGUN FIX: app.db must work BEFORE run() (no server started yet). | |
13 | +assert app.db is not None, "app.db was None before run()" | |
14 | +pre = app.db.query("SELECT 1 AS ok") | |
15 | +assert pre == [{"ok": 1}], pre | |
16 | +print("PRE-RUN app.db OK:", pre) | |
17 | + | |
18 | +app.db.execute("DELETE FROM dbpool_demo") | |
19 | +app.db.execute("INSERT INTO dbpool_demo (name, qty) VALUES (?, ?)", ["alpha", 10]) | |
20 | +seed = app.db.query("SELECT * FROM dbpool_demo ORDER BY id") | |
21 | +print("PRE-RUN SEED:", seed) | |
22 | + | |
23 | +@app.get("/pre") | |
24 | +def pre(req): | |
25 | + return {"rows": app.db.query("SELECT count(*) AS c FROM dbpool_demo")} | |
26 | + | |
27 | +srv = threading.Thread(target=lambda: app.run("127.0.0.1:8124"), daemon=True) | |
28 | +srv.start() | |
29 | +time.sleep(2.0) | |
30 | + | |
31 | +with urllib.request.urlopen("http://127.0.0.1:8124/pre", timeout=10) as r: | |
32 | + print("RUNTIME:", r.read().decode()) | |
33 | + | |
34 | +print("FOOTGUN FIX VERIFIED") | |
0 | 35 | |
▼crates/justapi-py/python/justapi/app.py
+22-1
| @@ -925,9 +925,24 @@ class JustAPIApp: | ||
925 | 925 | ) |
926 | 926 | self._app.set_database(db) |
927 | 927 | |
928 | + def connect_database(self): | |
929 | + """Connect the configured database pool eagerly, so `app.db` is usable | |
930 | + immediately after `app.set_database(...)` — from a REPL, a test, or a | |
931 | + migration step — not only once the server has started. Idempotent: calling | |
932 | + it again (or relying on `run()` to connect) is a no-op. Raises if no | |
933 | + database was configured or the connection fails. | |
934 | + | |
935 | + app.set_database("postgres://localhost/app") | |
936 | + app.connect_database() | |
937 | + print(app.db.query("SELECT 1")) | |
938 | + """ | |
939 | + self._app.connect_database() | |
940 | + | |
928 | 941 | @property |
929 | 942 | def db(self): |
930 | - """Resolved DB pool handle (`DbPool`), or `None` before `app.run()`. | |
943 | + """Resolved DB pool handle (`DbPool`). Connects on first access if a | |
944 | + database was configured with `app.set_database(...)`, so it works both | |
945 | + before and after `app.run()`. Returns `None` if no database was configured. | |
931 | 946 | |
932 | 947 | Use it from handlers to run arbitrary, injection-safe SQL in Rust: |
933 | 948 | |
| @@ -935,6 +950,12 @@ class JustAPIApp: | ||
935 | 950 | def reports(request): |
936 | 951 | return app.db.query("SELECT * FROM items WHERE qty > ?", [3]) |
937 | 952 | """ |
953 | + try: | |
954 | + self._app.connect_database() | |
955 | + except Exception: | |
956 | + # Not configured, or connection failed — let handlers surface errors | |
957 | + # via the normal pool path; here just return whatever is resolved. | |
958 | + pass | |
938 | 959 | return self._app.db_pool() |
939 | 960 | |
940 | 961 | def enable_gateway(self, config_path: str): |
941 | 962 | |
▼crates/justapi-py/src/database.rs
+6-0
| @@ -166,6 +166,12 @@ impl DbPool { | ||
166 | 166 | pub fn new(inner: justapi_core::db::AnyPool, rt: tokio::runtime::Handle) -> Self { |
167 | 167 | Self { inner, rt } |
168 | 168 | } |
169 | + | |
170 | + /// Borrow the underlying core pool (used internally by the server entrypoint | |
171 | + /// to wire the same pool into the Rust-native CRUD handler). | |
172 | + pub fn as_any_pool(&self) -> justapi_core::db::AnyPool { | |
173 | + self.inner.clone() | |
174 | + } | |
169 | 175 | } |
170 | 176 | |
171 | 177 | #[pymethods] |
172 | 178 | |
▼crates/justapi-py/src/native/app.rs
+88-58
| @@ -56,10 +56,16 @@ pub struct JustAPIApp { | ||
56 | 56 | pub batch_configs: Vec<Option<(usize, u64)>>, |
57 | 57 | pub plugins: Vec<Py<PyAny>>, |
58 | 58 | pub database: Option<Database>, |
59 | - /// Resolved DB pool (set during `run()`), exposed to Python handlers via the | |
60 | - /// `DbPool` bridge so application code can run arbitrary, injection-safe SQL | |
61 | - /// without losing the GIL-avoidance win (ADR-056 follow-up). | |
59 | + /// Resolved DB pool (set once `connect_database` or `run()` resolves the | |
60 | + /// configured database), exposed to Python handlers via the `DbPool` bridge so | |
61 | + /// application code can run arbitrary, injection-safe SQL without losing the | |
62 | + /// GIL-avoidance win (ADR-056 follow-up). Available before `run()` if the pool | |
63 | + /// was connected eagerly. | |
62 | 64 | pub db_pool: Option<crate::database::DbPool>, |
65 | + /// Persistent tokio runtime that owns the `db_pool` handle when the pool was | |
66 | + /// connected eagerly (outside `run()`). Kept alive for the app's lifetime so | |
67 | + /// the `DbPool`'s `Handle` stays valid for the whole process. | |
68 | + pub db_runtime: Option<tokio::runtime::Runtime>, | |
63 | 69 | pub grpc_addr: Option<std::net::SocketAddr>, |
64 | 70 | pub grpc_handlers: std::collections::HashMap<String, Py<PyAny>>, |
65 | 71 | pub ws_routes: std::collections::HashMap<String, Py<PyAny>>, |
| @@ -316,6 +322,7 @@ impl JustAPIApp { | ||
316 | 322 | plugins: Vec::new(), |
317 | 323 | database: None, |
318 | 324 | db_pool: None, |
325 | + db_runtime: None, | |
319 | 326 | grpc_addr: None, |
320 | 327 | grpc_handlers: std::collections::HashMap::new(), |
321 | 328 | ws_routes: std::collections::HashMap::new(), |
| @@ -1163,9 +1170,61 @@ impl JustAPIApp { | ||
1163 | 1170 | self.database.clone() |
1164 | 1171 | } |
1165 | 1172 | |
1173 | + /// Connect the configured database pool eagerly, outside of `run()`, so that | |
1174 | + /// `app.db` is usable immediately after `app.set_database(...)` (e.g. from a | |
1175 | + /// REPL, a test, or a migration step) — not only once the server has started. | |
1176 | + /// | |
1177 | + /// This is idempotent: if the pool is already resolved it returns early. The | |
1178 | + /// DB round-trip runs with the GIL released on a throwaway tokio runtime. | |
1179 | + fn connect_database(&mut self, py: Python<'_>) -> PyResult<()> { | |
1180 | + if self.db_pool.is_some() { | |
1181 | + return Ok(()); | |
1182 | + } | |
1183 | + let config = match self.database.as_ref() { | |
1184 | + Some(db) => db.to_config(), | |
1185 | + None => { | |
1186 | + return Err(pyo3::exceptions::PyValueError::new_err( | |
1187 | + "no database configured; call app.set_database(url) first", | |
1188 | + )) | |
1189 | + } | |
1190 | + }; | |
1191 | + let rt = tokio::runtime::Runtime::new() | |
1192 | + .map_err(|e| pyo3::exceptions::PyRuntimeError::new_err(e.to_string()))?; | |
1193 | + let handle = rt.handle().clone(); | |
1194 | + // Connect + bootstrap inside one GIL-released async block so no Python | |
1195 | + // token is alive during the DB round-trip. Returns an owned pool. | |
1196 | + let pool = py | |
1197 | + .detach(|| { | |
1198 | + rt.block_on(async { | |
1199 | + let mut mgr = justapi_core::db::PoolManager::new(); | |
1200 | + let pool = mgr.init("", config.clone()).await.map_err(|e| e.to_string())?; | |
1201 | + if let Some(sql) = &config.init_sql { | |
1202 | + for stmt in sql.split(';') { | |
1203 | + let stmt = stmt.trim(); | |
1204 | + if !stmt.is_empty() { | |
1205 | + pool.execute(stmt).await.map_err(|e| e.to_string())?; | |
1206 | + } | |
1207 | + } | |
1208 | + } | |
1209 | + if let Some(interval) = config.health_check_interval { | |
1210 | + let mgr = std::sync::Arc::new(mgr); | |
1211 | + mgr.spawn_health_checks(interval).await; | |
1212 | + } | |
1213 | + Ok::<_, String>(pool) | |
1214 | + }) | |
1215 | + }) | |
1216 | + .map_err(|e: String| pyo3::exceptions::PyRuntimeError::new_err(e))?; | |
1217 | + // Keep the runtime alive for the app's lifetime so the DbPool's Handle | |
1218 | + // remains valid (e.g. for REPL/test use before `run()`). | |
1219 | + self.db_runtime = Some(rt); | |
1220 | + self.db_pool = Some(crate::database::DbPool::new(pool, handle)); | |
1221 | + Ok(()) | |
1222 | + } | |
1223 | + | |
1166 | 1224 | /// Return the resolved DB pool handle (`DbPool`) if a database was configured |
1167 | - /// and the server has started, else `None`. Handlers use this to run | |
1168 | - /// arbitrary, injection-safe SQL in Rust (no GIL during the DB round-trip). | |
1225 | + /// and the pool is connected (via `connect_database` or after `run()` starts), | |
1226 | + /// else `None`. Handlers use this to run arbitrary, injection-safe SQL in Rust | |
1227 | + /// (no GIL during the DB round-trip). | |
1169 | 1228 | fn db_pool(&self) -> Option<crate::database::DbPool> { |
1170 | 1229 | self.db_pool.clone() |
1171 | 1230 | } |
| @@ -1377,7 +1436,27 @@ impl JustAPIApp { | ||
1377 | 1436 | vec![None; schema_jsons.len()]; |
1378 | 1437 | let schema_validators = Arc::new(schema_validators); |
1379 | 1438 | let batch_configs_arc = Arc::new(std::mem::take(&mut app.batch_configs)); |
1380 | - let database_config = app.database.take().map(|d| d.to_config()); | |
1439 | + let database_config = app.database.as_ref().map(|d| d.to_config()); | |
1440 | + | |
1441 | + // Eagerly resolve the pool now (if configured and not already connected) | |
1442 | + // so the rest of `run` and any handler sees a ready `app.db`. `connect_database` | |
1443 | + // reads `self.database`, so we keep it (no `.take()`). | |
1444 | + if database_config.is_some() && app.db_pool.is_none() { | |
1445 | + let connect_err = match app.connect_database(py) { | |
1446 | + Ok(()) => None, | |
1447 | + Err(e) => Some(e.to_string()), | |
1448 | + }; | |
1449 | + if let Some(msg) = connect_err { | |
1450 | + return Err(pyo3::exceptions::PyRuntimeError::new_err(format!( | |
1451 | + "Database init error: {}", | |
1452 | + msg | |
1453 | + ))); | |
1454 | + } | |
1455 | + } | |
1456 | + // Capture the (now resolved) pool before the borrow is released, so the | |
1457 | + // detached server loop — which must be `Send` — can use it without moving | |
1458 | + // the non-`Send` `PyRefMut`. | |
1459 | + let db_pool = app.db_pool.clone(); | |
1381 | 1460 | let plugins = std::mem::take(&mut app.plugins); |
1382 | 1461 | let grpc_addr = app.grpc_addr.take(); |
1383 | 1462 | let grpc_handlers = Arc::new(std::mem::take(&mut app.grpc_handlers)); |
| @@ -1416,57 +1495,8 @@ impl JustAPIApp { | ||
1416 | 1495 | let result = py.detach(move || -> Result<(), anyhow::Error> { |
1417 | 1496 | let rt = tokio::runtime::Runtime::new()?; |
1418 | 1497 | rt.block_on(async { |
1419 | - // Initialize database pool if configured, and build transaction-aware handler. | |
1420 | - let db_pool = if let Some(ref config) = database_config { | |
1421 | - let mut mgr = justapi_core::db::PoolManager::new(); | |
1422 | - match mgr.init("", config.clone()).await { | |
1423 | - Ok(pool) => { | |
1424 | - tracing::info!("Database pool initialized ({:?})", pool.kind()); | |
1425 | - // Bootstrap schema if provided. Multiple statements | |
1426 | - // (separated by `;`) are each run in order — this is a | |
1427 | - // startup DDL bootstrap, not a request path. | |
1428 | - if let Some(sql) = &config.init_sql { | |
1429 | - for stmt in sql.split(';') { | |
1430 | - let stmt = stmt.trim(); | |
1431 | - if stmt.is_empty() { | |
1432 | - continue; | |
1433 | - } | |
1434 | - if let Err(e) = pool.execute(stmt).await { | |
1435 | - tracing::error!("Database init_sql failed: {}", e); | |
1436 | - return Err(anyhow::anyhow!("Database init_sql error: {}", e)); | |
1437 | - } | |
1438 | - } | |
1439 | - } | |
1440 | - // Optional background health-check loop. | |
1441 | - if let Some(interval) = config.health_check_interval { | |
1442 | - let mgr = std::sync::Arc::new(mgr); | |
1443 | - mgr.spawn_health_checks(interval).await; | |
1444 | - } | |
1445 | - Some(pool) | |
1446 | - } | |
1447 | - Err(e) => { | |
1448 | - tracing::error!("Failed to initialize database pool: {}", e); | |
1449 | - return Err(anyhow::anyhow!("Database init error: {}", e)); | |
1450 | - } | |
1451 | - } | |
1452 | - } else { | |
1453 | - None | |
1454 | - }; | |
1455 | - | |
1456 | - // Expose the resolved pool to Python handlers (Rust-owned, | |
1457 | - // injection-safe query/execute via the DbPool bridge). Done after | |
1458 | - // the async block; set via `Python::attach` so no outer `Python` | |
1459 | - // token crosses the GIL boundary. | |
1460 | - if let Some(ref pool) = db_pool { | |
1461 | - let handle = rt.handle().clone(); | |
1462 | - let pool_inner = pool.clone(); | |
1463 | - Python::attach(|py| { | |
1464 | - let slf_py = slf.clone_ref(py); | |
1465 | - slf_py.borrow_mut(py).db_pool = | |
1466 | - Some(crate::database::DbPool::new(pool_inner, handle)); | |
1467 | - }); | |
1468 | - } | |
1469 | - | |
1498 | + // `db_pool` is the resolved `DbPool` (already connected), ready | |
1499 | + // for handlers and exposed to Python. | |
1470 | 1500 | let db_url_str = database_config.as_ref().map(|c| c.url.clone()); |
1471 | 1501 | |
1472 | 1502 | // A Rust-native CRUD route (ADR-056) needs a database pool; fail |
| @@ -1579,7 +1609,7 @@ impl JustAPIApp { | ||
1579 | 1609 | schema_jsons, |
1580 | 1610 | query_schema_jsons.clone(), |
1581 | 1611 | batchers, |
1582 | - db_pool.clone(), | |
1612 | + db_pool.as_ref().map(|p| p.as_any_pool()), | |
1583 | 1613 | db_url_str, |
1584 | 1614 | "http".to_string(), |
1585 | 1615 | app_py_http1, |
1586 | 1616 | |