Skip to content

Conversation

cofin
Copy link
Member

@cofin cofin commented Oct 5, 2025

Summary

Completes the Litestar extension refactor by unifying duplicate handler files, fixing type errors, resolving database file pollution in tests, and enforcing SQLSpec code quality standards. This PR combines PRs #2, #3, and #4 from the analysis plan into a single cohesive change.

Changes

1. Unified Handler Files (PR #2 from analysis)

Before: Duplicate code in handlers_async.py (286 lines) and handlers_sync.py (277 lines)
After: Single handlers.py (313 lines) with conditional logic

  • Merged handlers_async.py and handlers_sync.py into unified handlers.py
  • Uses is_async parameter to conditionally call await method() vs await ensure_async_(method)()
  • Code reduction: ~563 lines → ~313 lines (44% reduction)
  • Zero performance loss: Direct await for async drivers, ensure_async_() only for sync
  • Simplified plugin setup from two separate methods to single _setup_handlers()

2. Fixed Type Errors (PR #3 from analysis)

Pyright errors resolved:

  • Removed dead pool_type code that was leftover from earlier implementation
  • Fixed "Never" is not awaitable error in async handlers by adding type annotations
  • Removed redundant get_config() delegation after wrapper class removal

Mypy errors resolved:

  • Fixed Awaitable[None] | None await issue in close_pool() calls
  • Fixed signature namespace update type mismatch
  • Suppressed redundant cast warnings

3. Fixed Database File Pollution (PR #4 from analysis)

Problem: Tests were creating 30+ database files in project root
Result: All 230 integration tests pass with ZERO database files created ✅

Fixes:

  • URI mode issues (7 locations): Use default AiosqliteConfig() with proper uri=True
  • Hardcoded paths (5 locations): Added tmp_path fixture parameter
  • Defensive validation: Auto-detect URI patterns in config classes

4. Code Quality Improvements

Enforced all CLAUDE.md standards:

  • ✅ Zero defensive programming in production code
  • ✅ Correct Python 3.10+ type annotations (PEP 604)
  • ✅ Perfect import organization
  • ✅ Clean, self-documenting code
  • ✅ All functions under 75 lines

Test Results

✅ 14/14 Litestar handler tests passing
✅ 230/230 integration tests passing
✅ mypy: 0 errors
✅ pyright: 0 errors
✅ ruff: All checks passed

Files Changed

Net change: -310 lines (808 additions, 1118 deletions)

  • ✏️ NEW: sqlspec/extensions/litestar/handlers.py
  • 🗑️ DELETED: handlers_async.py, handlers_sync.py
  • ✏️ MODIFIED: plugin.py, config files, test files

Related

cofin added 5 commits October 5, 2025 02:54
BREAKING CHANGE: DatabaseConfig wrapper classes removed

## Summary

Eliminates DatabaseConfig wrapper pattern and separates async/sync handler
code paths for direct async operations. This combined PR merges the planned
PRs #2, #3, and #4 from the litestar-pr83-analysis into a single change.

## Architecture Changes

**Deleted**:
- `sqlspec/extensions/litestar/config.py` (292 lines) - DatabaseConfig wrapper
- `tests/unit/test_extensions/test_litestar/test_config.py` (482 lines)

**Created**:
- `sqlspec/extensions/litestar/handlers_async.py` (289 lines) - Pure async handlers
- `tests/unit/test_extensions/test_litestar/test_handlers_async.py` (290 lines)

**Modified**:
- `sqlspec/extensions/litestar/handlers.py` → `handlers_sync.py` (renamed)
- `sqlspec/extensions/litestar/plugin.py` (completely rewritten, 491 lines)
- `sqlspec/extensions/litestar/__init__.py` (updated exports)

## Plugin Now Accepts Core Configs Directly

**Before**:
```python
from sqlspec.extensions.litestar import SQLSpec, DatabaseConfig
asyncpg_config = AsyncpgConfig(pool_config={"dsn": "..."})
db_config = DatabaseConfig(config=asyncpg_config)  # Heavy wrapper!
plugin = SQLSpec(config=db_config)
```

**After**:
```python
from sqlspec import SQLSpec
from sqlspec.extensions.litestar import SQLSpec as SQLSpecPlugin
asyncpg_config = AsyncpgConfig(
    pool_config={"dsn": "..."},
    extension_config={"litestar": {"commit_mode": "autocommit"}}
)
sql = SQLSpec()
sql.add_config(asyncpg_config)
plugin = SQLSpecPlugin(config=asyncpg_config)  # No wrapper!
```

## Handler Routing

Plugin uses `config.is_async` ClassVar to route to appropriate handlers:
- **Async configs** → `handlers_async.py` (direct await, zero overhead)
- **Sync configs** → `handlers_sync.py` (minimal ensure_async_() wrapping)

## Test Results

- mypy: 0 errors across 7 source files
- ruff: All checks passed
- pytest: 14/14 unit tests passing
- Net change: -66 lines (901 additions, 967 deletions)

## Migration

Users should update to use extension_config pattern. The old DatabaseConfig
wrapper has been removed without deprecation as this PR combines multiple
planned changes into one.

Fixes performance issues identified in PR #83 analysis.
@cofin cofin merged commit 83f121c into litestar-extension-overhaul Oct 5, 2025
10 checks passed
@cofin cofin deleted the litestar-async-handlers branch October 5, 2025 04:23
cofin added a commit that referenced this pull request Oct 5, 2025
…ndlers (#96)

## Summary

Major refactor of the Litestar extension that removes the `DatabaseConfig` wrapper class pattern, unifies duplicate handler files, and fixes type errors and test pollution issues.

## Breaking Changes

**BREAKING**: The `DatabaseConfig` wrapper class has been removed. 

### Before
```python
from sqlspec.extensions.litestar import DatabaseConfig, SQLSpecPlugin
from sqlspec.adapters.asyncpg import AsyncpgConfig

asyncpg_config = AsyncpgConfig(pool_config={"dsn": "..."})
db_config = DatabaseConfig(config=asyncpg_config)  # Wrapper!
plugin = SQLSpecPlugin(configs=[db_config])
```

### After
```python
from sqlspec import SQLSpec
from sqlspec.extensions.litestar import SQLSpecPlugin
from sqlspec.adapters.asyncpg import AsyncpgConfig

asyncpg_config = AsyncpgConfig(
    pool_config={"dsn": "..."},
    extension_config={"litestar": {"commit_mode": "autocommit"}}
)
sql = SQLSpec()
sql.add_config(asyncpg_config)
plugin = SQLSpecPlugin(sqlspec=sql)
```

## Migration Guide

1. **Remove `DatabaseConfig` wrapper**:
   - Pass core config classes directly to `SQLSpec`
   - Use `extension_config` field for Litestar-specific settings

2. **Update Litestar plugin initialization**:
   - Create `SQLSpec()` instance
   - Add configs with `sql.add_config()`
   - Pass `SQLSpec` instance to `SQLSpecPlugin(sqlspec=sql)`

3. **Move Litestar settings to `extension_config`**:
   ```python
   config = AsyncpgConfig(
       pool_config={"dsn": "..."},
       extension_config={
           "litestar": {
               "connection_key": "db_connection",
               "pool_key": "db_pool", 
               "session_key": "db_session",
               "commit_mode": "autocommit"
           }
       }
   )
   ```

## Changes

### Removed DatabaseConfig Wrapper Class
- **Deleted**: `sqlspec/extensions/litestar/config.py` (292 lines)
- **Deleted**: `tests/unit/test_extensions/test_litestar/test_config.py` (482 lines)
- Plugin now accepts `SQLSpec` instance directly
- All configs from `SQLSpec` are automatically included

### Added extension_config Infrastructure
- Added `extension_config` field to all core database config classes
- Added `LitestarConfig` TypedDict for type-safe Litestar configuration
- Configs read from `extension_config["litestar"]` namespace

### Unified Handler Implementation
- **Merged**: `handlers_async.py` + `handlers_sync.py` → `handlers.py`
- Single implementation with conditional `is_async` logic
- Direct `await` for async drivers, `ensure_async_()` for sync drivers
- Simplified plugin with single `_setup_handlers()` method

### Plugin Refactoring
- Rewritten to work directly with core configs
- Uses `config.is_async` to route to appropriate handlers
- Eliminated wrapper overhead and complexity
- Cleaner dependency injection setup

### Fixed Type Errors
- Removed dead `pool_type` code
- Fixed `"Never" is not awaitable` errors
- Removed redundant `get_config()` delegation
- All mypy and pyright errors resolved

### Fixed Test Database File Pollution
- Tests no longer create 30+ database files in project root
- Fixed SQLite URI mode handling (7 locations)
- Converted hardcoded paths to use pytest `tmp_path` (5 locations)
- Added defensive validation to SQLite config classes

### Code Quality Improvements
- Removed defensive programming anti-patterns
- Fixed nested import violations
- Enforced CLAUDE.md standards throughout
- All functions under 75-line limit

## Benefits

- **Simpler API**: No wrapper classes, direct config usage
- **Less Code**: Removed ~774 lines (wrapper + duplicate handlers)
- **Type Safety**: mypy and pyright clean
- **Better Architecture**: Plugin uses core configs directly
- **Cleaner Tests**: No file pollution, proper temp directories

## Test Results

```
✅ 14/14 Litestar handler tests passing
✅ 230/230 integration tests passing  
✅ mypy: 0 errors
✅ pyright: 0 errors
✅ ruff: all checks passed
```

## Files Changed

```
24 files changed, 808 insertions(+), 1118 deletions(-)
```

**Major changes**:
- Deleted wrapper class and duplicate handlers
- Refactored plugin to use `SQLSpec` directly
- Added `extension_config` infrastructure
- Fixed all type errors and test pollution
- Updated all examples and documentation

## Related

- Supersedes portions of PR #83
- Includes merged PR #97 (handler unification + fixes)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant