Skip to content

Commit 7e8c255

Browse files
authored
fix: session key consistency (#152)
Ensure naming convention consistency across all extensions.
1 parent 8b67870 commit 7e8c255

File tree

12 files changed

+768
-28
lines changed

12 files changed

+768
-28
lines changed

AGENTS.md

Lines changed: 146 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -229,17 +229,20 @@ def test_starlette_autocommit_mode() -> None:
229229
```
230230

231231
**Why this works**:
232+
232233
- Each test creates a unique temporary file
233234
- No database state shared between tests
234235
- Tests can run in parallel safely with `pytest -n 2 --dist=loadgroup`
235236
- Files automatically deleted on test completion
236237

237238
**When to use**:
239+
238240
- Framework extension tests (Starlette, FastAPI, Flask, etc.)
239241
- Any test using connection pooling with SQLite
240242
- Integration tests that run in parallel
241243

242244
**Alternatives NOT recommended**:
245+
243246
- `CREATE TABLE IF NOT EXISTS` - Masks test isolation issues
244247
- Disabling pooling - Tests don't reflect production configuration
245248
- Running tests serially - Slows down CI significantly
@@ -1446,11 +1449,58 @@ def _teardown_appcontext_handler(self, _exc: "Exception | None" = None) -> None:
14461449
```
14471450

14481451
**Key differences from Starlette/FastAPI**:
1452+
14491453
- **No middleware**: Use `before_request`, `after_request`, `teardown_appcontext` hooks
14501454
- **Flask g object**: Store connections/sessions on `g` (request-scoped global)
14511455
- **Must return response**: `after_request` hook must return the response unchanged
14521456
- **Nested imports required**: Import `flask.g` and `flask.current_app` inside hooks to access request context
14531457

1458+
### DEFAULT_SESSION_KEY Standardization
1459+
1460+
All framework extensions MUST use the same default session key for consistency:
1461+
1462+
```python
1463+
# MANDATORY: All framework extensions use "db_session"
1464+
DEFAULT_SESSION_KEY = "db_session"
1465+
```
1466+
1467+
**Why "db_session"**:
1468+
1469+
- Used by Litestar as dependency injection key name
1470+
- Changing would break Litestar's DI system
1471+
- Consistency across frameworks reduces cognitive overhead
1472+
- More descriptive than alternatives like "default"
1473+
1474+
**Where it's used**:
1475+
1476+
- `sqlspec/extensions/flask/extension.py:22`
1477+
- `sqlspec/extensions/starlette/extension.py:24`
1478+
- `sqlspec/extensions/litestar/plugin.py:50`
1479+
1480+
**Pattern for multi-database setups**:
1481+
1482+
```python
1483+
# Primary database uses default key
1484+
primary_config = AsyncpgConfig(
1485+
pool_config={"dsn": "postgresql://localhost/main"},
1486+
extension_config={
1487+
"starlette": {"session_key": "db_session", "commit_mode": "autocommit"}
1488+
}
1489+
)
1490+
1491+
# Secondary database uses custom key
1492+
analytics_config = SqliteConfig(
1493+
pool_config={"database": "analytics.db"},
1494+
extension_config={
1495+
"starlette": {"session_key": "analytics", "commit_mode": "manual"}
1496+
}
1497+
)
1498+
1499+
# Access by key
1500+
db = plugin.get_session() # Uses "db_session" (default)
1501+
analytics = plugin.get_session("analytics") # Uses custom key
1502+
```
1503+
14541504
### Portal Pattern for Sync Frameworks
14551505

14561506
Enable async adapters (asyncpg, asyncmy, aiosqlite) in sync WSGI frameworks:
@@ -1500,6 +1550,98 @@ else:
15001550

15011551
**Performance**: ~1-2ms overhead per operation. **Recommended**: Use sync adapters for Flask.
15021552

1553+
### Flask Pool and Portal Cleanup Pattern
1554+
1555+
Flask requires explicit resource cleanup using Python's `atexit` module:
1556+
1557+
```python
1558+
import atexit
1559+
1560+
class SQLSpecPlugin:
1561+
def init_app(self, app: "Flask") -> None:
1562+
"""Initialize Flask application with SQLSpec."""
1563+
# Create pools
1564+
pools: dict[str, Any] = {}
1565+
for config_state in self._config_states:
1566+
if config_state.config.supports_connection_pooling:
1567+
if config_state.is_async:
1568+
pool = self._portal.portal.call(config_state.config.create_pool)
1569+
else:
1570+
pool = config_state.config.create_pool()
1571+
pools[config_state.session_key] = pool
1572+
1573+
# Register cleanup hook
1574+
self._register_shutdown_hook()
1575+
1576+
def _register_shutdown_hook(self) -> None:
1577+
"""Register shutdown hook for pool and portal cleanup."""
1578+
if self._cleanup_registered:
1579+
return
1580+
1581+
atexit.register(self.shutdown)
1582+
self._cleanup_registered = True
1583+
1584+
def shutdown(self) -> None:
1585+
"""Dispose connection pools and stop async portal."""
1586+
if self._shutdown_complete:
1587+
return
1588+
1589+
self._shutdown_complete = True
1590+
1591+
# Close all pools
1592+
for config_state in self._config_states:
1593+
if config_state.config.supports_connection_pooling:
1594+
try:
1595+
if config_state.is_async:
1596+
self._portal.portal.call(config_state.config.close_pool)
1597+
else:
1598+
config_state.config.close_pool()
1599+
except Exception:
1600+
logger.exception("Error closing pool during shutdown")
1601+
1602+
# Stop portal
1603+
if self._portal is not None:
1604+
try:
1605+
self._portal.stop()
1606+
except Exception:
1607+
logger.exception("Error stopping portal during shutdown")
1608+
finally:
1609+
self._portal = None
1610+
```
1611+
1612+
**Key requirements**:
1613+
1614+
- **Idempotent**: Use `_shutdown_complete` flag to prevent double cleanup
1615+
- **Single registration**: Use `_cleanup_registered` flag to avoid multiple atexit hooks
1616+
- **Error handling**: Log exceptions but don't fail on cleanup errors
1617+
- **Portal cleanup**: Always stop portal for async configs to prevent thread leaks
1618+
- **Sync and async**: Handle both sync pool cleanup and async pool cleanup via portal
1619+
1620+
**Why atexit**:
1621+
1622+
- Flask (WSGI) lacks native lifecycle context managers
1623+
- atexit ensures cleanup on interpreter shutdown
1624+
- Works with development servers, production servers, and tests
1625+
- Standard pattern for library resource cleanup
1626+
1627+
**Testing pattern**:
1628+
1629+
```python
1630+
def test_flask_pool_cleanup():
1631+
"""Verify pools are cleaned up on shutdown."""
1632+
plugin = SQLSpecPlugin(sqlspec, app)
1633+
1634+
# Verify atexit hook registered
1635+
assert plugin._cleanup_registered
1636+
1637+
# Trigger shutdown
1638+
plugin.shutdown()
1639+
1640+
# Verify cleanup complete
1641+
assert plugin._shutdown_complete
1642+
assert plugin._portal is None
1643+
```
1644+
15031645
### HTTP Status Code Constants
15041646

15051647
Avoid magic values by defining module-level constants:
@@ -1561,6 +1703,7 @@ result = portal.call(some_async_function, arg1, arg2)
15611703
```
15621704

15631705
**Benefits**:
1706+
15641707
- Available to Django, Bottle, or any sync framework
15651708
- Single import location for all portal functionality
15661709
- Thread-safe singleton for automatic portal management
@@ -1603,6 +1746,7 @@ def await_(async_function, raise_sync_error=False): # Portal enabled by default
16031746
```
16041747

16051748
**Benefits**:
1749+
16061750
- **Transparent async support by default** - no parameter needed
16071751
- No manual portal management required
16081752
- Global portal automatically created and reused
@@ -1634,10 +1778,12 @@ class PortalManager(metaclass=SingletonMeta):
16341778
```
16351779

16361780
**Key Features**:
1781+
16371782
- Uses `SingletonMeta` for thread-safe singleton pattern
16381783
- Double-checked locking for performance
16391784
- Lazy initialization - portal only created when needed
16401785
- Global portal shared across all `await_()` calls
1786+
16411787
## Framework Extension Pattern (Middleware-Based)
16421788

16431789
### Overview

0 commit comments

Comments
 (0)