From 9e46bdc460eb398a60bfa91704532168adfb74e8 Mon Sep 17 00:00:00 2001 From: Barcode Betty Date: Wed, 3 Jun 2026 12:16:03 +0000 Subject: [PATCH 1/3] fix(api): document dispose_engine lazy import + regression test (CAR-1135) - main.py: add docstring inside the lifespan function explaining why dispose_engine is lazy-imported rather than top-level. The original import path (top-level) crashed the container at import time with 'ImportError: cannot import name dispose_engine from cartsnitch_api.database' when database.py was stale or stripped during a CI build. Lazy import keeps the engine disposal behavior while preventing the module-load crash. - tests/test_openapi.py: add test_dispose_engine_importable_from_database that asserts dispose_engine is importable and callable. This is the exact path the deployed UAT image was failing on, captured as a regression test so a future regression lands in CI before deploy. Refs CAR-1135. Co-Authored-By: Paperclip --- src/cartsnitch_api/main.py | 6 ++++++ tests/test_openapi.py | 13 +++++++++++++ 2 files changed, 19 insertions(+) diff --git a/src/cartsnitch_api/main.py b/src/cartsnitch_api/main.py index 5a72b6b..66c1854 100644 --- a/src/cartsnitch_api/main.py +++ b/src/cartsnitch_api/main.py @@ -25,6 +25,12 @@ from cartsnitch_api.routes.user import router as user_router @asynccontextmanager async def lifespan(app: FastAPI): + # Lazy import: keep `dispose_engine` out of the top-level imports so a + # stale or partially-built database.py never breaks module load on + # container start. The function is required for graceful pool cleanup + # on shutdown; if the import fails, the cache_client.close() that + # follows the yield would mask it. See CAR-1135 for the original + # ImportError that motivated this pattern. from cartsnitch_api.database import dispose_engine await cache_client.initialize() diff --git a/tests/test_openapi.py b/tests/test_openapi.py index 2311567..d450430 100644 --- a/tests/test_openapi.py +++ b/tests/test_openapi.py @@ -3,8 +3,21 @@ import pytest from httpx import ASGITransport, AsyncClient +from cartsnitch_api.database import dispose_engine from cartsnitch_api.main import app + +def test_dispose_engine_importable_from_database(): + """Regression for CAR-1135: api main.py used to import dispose_engine + at module level. A stale database.py (no dispose_engine) crashed the + container at import time with ImportError on line 9. The fix moved + the import inside the lifespan function, but `dispose_engine` must + still be importable from `cartsnitch_api.database` for the lifespan + teardown to actually close pooled connections. + """ + assert callable(dispose_engine) + assert dispose_engine.__name__ == "dispose_engine" + EXPECTED_ROUTES = [ # Auth (3 — register/login/refresh are handled by Better-Auth service) ("get", "/auth/me"), -- 2.52.0 From 4877513bbf3dccb35e42ec5c3d62fbf4c0ee2f55 Mon Sep 17 00:00:00 2001 From: Barcode Betty Date: Tue, 9 Jun 2026 05:23:36 +0000 Subject: [PATCH 2/3] style: ruff format conformance (CAR-1335) - tests/test_openapi.py: collapse 2 blank lines to 1 (ruff format) - tests/conftest.py: collapse 2 blank lines to 1 (ruff format) These format nits block lint (a hard gate). The conftest.py one was introduced in CAR-1132 (#42) and would have blocked every subsequent PR on dev until fixed. Refs CAR-1335, CAR-1135. Co-Authored-By: Paperclip --- tests/conftest.py | 1 - tests/test_openapi.py | 1 + 2 files changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/conftest.py b/tests/conftest.py index 1958022..133f726 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -117,7 +117,6 @@ def _register_event_listeners(): event.listen(cls, "before_insert", _set_timestamp_defaults) - TEST_JWT_SECRET = secrets.token_urlsafe(32) TEST_SERVICE_KEY = secrets.token_urlsafe(32) TEST_FERNET_KEY = "7reF42nmTwbdN21PBoubGp7h_FU8qSimstmlaMLoRK8=" diff --git a/tests/test_openapi.py b/tests/test_openapi.py index d450430..1abea55 100644 --- a/tests/test_openapi.py +++ b/tests/test_openapi.py @@ -18,6 +18,7 @@ def test_dispose_engine_importable_from_database(): assert callable(dispose_engine) assert dispose_engine.__name__ == "dispose_engine" + EXPECTED_ROUTES = [ # Auth (3 — register/login/refresh are handled by Better-Auth service) ("get", "/auth/me"), -- 2.52.0 From 7b595744e1f36457635285ff1fadcb3197a6587d Mon Sep 17 00:00:00 2001 From: Barcode Betty Date: Tue, 9 Jun 2026 05:25:41 +0000 Subject: [PATCH 3/3] fix(api): mypy no-redef and no-any-return errors on dev (CAR-1335) The api typecheck job is continue-on-error but still posts a failure status that blocks merges. Three pre-existing mypy errors on dev were inherited by every PR based on it: 1. middleware/rate_limit.py: duplicate 'name already defined' for _public_limiter, _auth_limiter, _auth_strict_limiter (declared at lines 111-113 and again at 124-126). The second set is redundant because actual assignment happens inside the if/else below. 2. cache.py:43 - 'Returning Any' from .get(); the redis client's get() return type isn't narrowed to bytes|str, so the final 'return value' branch is Any. Wrap with str() to satisfy the declared str|None. 3. middleware/rate_limit.py:150 - 'Returning Any' from _get_client_ip. request.headers.get() and request.client.host are typed Any; wrap the branches with str() to match the declared str return. Refs CAR-1335. Co-Authored-By: Paperclip --- src/cartsnitch_api/cache.py | 2 +- src/cartsnitch_api/middleware/rate_limit.py | 8 ++------ 2 files changed, 3 insertions(+), 7 deletions(-) diff --git a/src/cartsnitch_api/cache.py b/src/cartsnitch_api/cache.py index 6766a8c..a5aa91b 100644 --- a/src/cartsnitch_api/cache.py +++ b/src/cartsnitch_api/cache.py @@ -40,7 +40,7 @@ class CacheClient: return None if isinstance(value, bytes): return value.decode("utf-8", errors="replace") - return value + return str(value) async def set(self, key: str, value: str, ttl_seconds: int = 300) -> None: if not self._client: diff --git a/src/cartsnitch_api/middleware/rate_limit.py b/src/cartsnitch_api/middleware/rate_limit.py index c6d5f21..ff1b218 100644 --- a/src/cartsnitch_api/middleware/rate_limit.py +++ b/src/cartsnitch_api/middleware/rate_limit.py @@ -121,10 +121,6 @@ if settings.rate_limit_redis_enabled: logger.warning("Failed to connect to Redis for rate limiting, using in-memory: %s", e) _use_redis = False -_public_limiter: RateLimitBackend -_auth_limiter: RateLimitBackend -_auth_strict_limiter: RateLimitBackend - if _use_redis and _redis_client: _public_limiter = RedisSlidingWindow( _redis_client, settings.rate_limit_requests, settings.rate_limit_window_seconds @@ -151,8 +147,8 @@ def _get_client_ip(request: Request) -> str: """Extract client IP, respecting X-Forwarded-For behind a reverse proxy.""" forwarded = request.headers.get("x-forwarded-for") if forwarded: - return forwarded.split(",")[0].strip() - return request.client.host if request.client else "unknown" + return str(forwarded.split(",")[0].strip()) + return str(request.client.host) if request.client else "unknown" def _get_rate_limit_key(request: Request) -> tuple[str, RateLimitBackend]: -- 2.52.0