- cache.py:38: Add explicit type annotation for redis.get() return value to resolve mypy no-any-return
- rate_limit.py: Remove duplicate forward-declaration block (dead code, mypy no-redef)
- conftest.py: Remove one excess blank line to satisfy ruff format check
All three fixes verified locally: ruff check ✅, ruff format ✅, mypy ✅
Co-Authored-By: Paperclip <noreply@paperclip.ing>
QA review of PR #39 (CAR-1121) identified three blocking issues; this
commit addresses all three plus the typecheck errors flagged as CI RED.
CAR-1077 (PR #39) changes:
- database.py: add pool_timeout=30 so the engine fails fast when the
connection pool is exhausted (defends against the "server closed
connection unexpectedly" pod failures).
- routes/health.py: /health now calls SELECT 1 through Depends(get_db)
and raises HTTPException(503) when the database is unreachable, so
Kubernetes readiness probes can correctly mark the pod unhealthy and
stop routing traffic to it. Logs the failure at exception level for
observability.
- Drop .mcp.json from this PR (root-level MCP server config, not
related to the pool fix; tracked separately).
CI typecheck fixes (pre-existing on dev, were failing mypy on PR #39):
- auth/passwords.py: cast bcrypt return values so mypy doesn't widen
to Any.
- config.py: silence the false-positive call-arg on Settings() — the
three required fields are populated from the environment by
pydantic-settings at runtime.
- cache.py: coerce the bytes/str union returned by the redis client
to the documented str | None return type.
- middleware/rate_limit.py: annotate the three module-level limiters
with the RateLimitBackend protocol, cast the redis zrange score to
float before arithmetic, and add max_requests/window_seconds to the
protocol so the response-header builder can read them.
Co-Authored-By: Paperclip <noreply@paperclip.ing>
mypy complained: 'Unsupported operand types for - ("str" and "float")'
on rate_limit.py:87. redis-py's zrange withscores=True returns the
score as whatever the codec produces (often str), but we treat it as
a numeric millisecond timestamp. Cast to float before subtracting
the cutoff.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Three categories of pre-existing CI failure on PR #42:
1. typecheck (mypy src/cartsnitch_api, 9 errors):
- src/cartsnitch_api/config.py:89 — Settings() needs required secret
args that only exist in env at runtime; suppress with
type: ignore[call-arg]
- src/cartsnitch_api/cache.py:38 — redis-py returns Any/bytes,
normalize to str before returning from get()
- src/cartsnitch_api/middleware/rate_limit.py:128,131,134 — three
limiter globals were inferred as RedisSlidingWindow on the if
branch then re-assigned InMemorySlidingWindow on else; declare
them as RateLimitBackend up front
- src/cartsnitch_api/middleware/rate_limit.py:181,187 —
RateLimitBackend Protocol didn't declare max_requests even
though both InMemorySlidingWindow and RedisSlidingWindow expose
it; add max_requests: int to the Protocol
2. test (FK constraint on purchases.user_id):
- tests/conftest.py:_create_test_user_and_session stored user_id
as 32-char hex; test_e2e conftest reads it via raw SQL and wraps
in uuid.UUID (36 chars) before passing to Purchase.user_id, so
the FK never matched. Switch back to str(uuid.uuid4()) (36 chars)
so the stored value and the FK bind value use the same format.
3. Verify lint + format clean.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
- Add rate_limit_auth_requests (5/min) and rate_limit_auth_window_seconds (60) settings
- Add rate_limit_redis_enabled flag for opt-in Redis usage
- Refactor _SlidingWindowCounter into InMemorySlidingWindow class
- Add RedisSlidingWindow using sorted sets with fallback to in-memory
- Add third _auth_strict_limiter for POST /auth/* paths (5 req/min)
- Add protocol-based backend selection at module load time
- Update tests for auth strict limiter and Redis fallback behavior
Co-Authored-By: Paperclip <noreply@paperclip.ing>
- Add rate_limit_auth_requests (5/min) and rate_limit_auth_window_seconds (60)
settings to config.py
- Refactor rate_limit.py to use protocol/ABC pattern with InMemorySlidingWindow
and RedisSlidingWindow implementations
- Add RedisSlidingWindow using sorted sets for distributed rate limiting
- Add auth_strict_limiter for /auth/* POST endpoints (5 req/min per IP)
- Fall back to in-memory when Redis is unavailable
- Update tests to cover new functionality
Co-Authored-By: Paperclip <noreply@paperclip.ing>