cartsnitch/api
12
0
Fork 1
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Fix SQLite CI test failures: UUID binding, func.now() defaults, F402 lint (CAR-1012) #40

Closed
Barcode Betty wants to merge 2 commits from betty/fix-ci-test-failures-v2 into dev
pull from: betty/fix-ci-test-failures-v2
merge into: cartsnitch:dev
Conversation 1 Commits 2 Files Changed 7
+96 -26
Barcode Betty commented 2026-05-30 09:09:36 +00:00
Member
Copy Link
Copy Source

Summary

Fixes three remaining CI test failures from PR #35 review:

  1. F402 lint: Loop variable table shadowed imported table in test_encrypted_json.py
  2. func.now() server_defaults: TimestampMixin and Purchase.ingested_at used PostgreSQL-only server_default=func.now() — stripped in fixtures AND given Python-side default=_utcnow
  3. UUID binding: SQLite can't bind Python uuid.UUID objects — added aiosqlite.register_adapter() for async engine and GuidType TypeDecorator for cross-DB UUID handling

Files changed

  • src/cartsnitch_api/types.py — new GuidType TypeDecorator (UUID to String)
  • src/cartsnitch_api/models/base.py — TimestampMixin adds default=_utcnow, UUIDPrimaryKeyMixin uses GuidType
  • src/cartsnitch_api/models/user.py — User.id uses GuidType() plus Mapped[uuid.UUID], email_verified adds default=False
  • src/cartsnitch_api/models/purchase.py — ingested_at adds default=_utcnow
  • tests/conftest.py — strips func.now() from server_defaults, adds aiosqlite.register_adapter(uuid.UUID, ...)
  • tests/test_encrypted_json.py — fixes F402, strips func.now()

Test results

  • All 6 test_encrypted_json.py tests pass
  • All 17 test_models.py tests pass
  • 55 passed / 116 failed total (116 route-level failures are pre-existing: routes mounted at /api/v1 but tests hit /stores without prefix — unrelated to this fix)

cc @cpfarhood

## Summary Fixes three remaining CI test failures from PR #35 review: 1. **F402 lint**: Loop variable `table` shadowed imported `table` in `test_encrypted_json.py` 2. **func.now() server_defaults**: `TimestampMixin` and `Purchase.ingested_at` used PostgreSQL-only `server_default=func.now()` — stripped in fixtures AND given Python-side `default=_utcnow` 3. **UUID binding**: SQLite can't bind Python `uuid.UUID` objects — added `aiosqlite.register_adapter()` for async engine and `GuidType` TypeDecorator for cross-DB UUID handling ## Files changed - `src/cartsnitch_api/types.py` — new `GuidType` TypeDecorator (UUID to String) - `src/cartsnitch_api/models/base.py` — `TimestampMixin` adds `default=_utcnow`, `UUIDPrimaryKeyMixin` uses `GuidType` - `src/cartsnitch_api/models/user.py` — `User.id` uses `GuidType()` plus `Mapped[uuid.UUID]`, `email_verified` adds `default=False` - `src/cartsnitch_api/models/purchase.py` — `ingested_at` adds `default=_utcnow` - `tests/conftest.py` — strips `func.now()` from server_defaults, adds `aiosqlite.register_adapter(uuid.UUID, ...)` - `tests/test_encrypted_json.py` — fixes F402, strips `func.now()` ## Test results - All 6 `test_encrypted_json.py` tests pass - All 17 `test_models.py` tests pass - 55 passed / 116 failed total (116 route-level failures are pre-existing: routes mounted at `/api/v1` but tests hit `/stores` without prefix — unrelated to this fix) cc @cpfarhood
Barcode Betty added 2 commits 2026-05-30 09:09:36 +00:00
Fix SQLite server_default AttributeError and pool_size errors
CI / lint (pull_request) Failing after 3s
Details
CI / typecheck (pull_request) Failing after 18s
Details
CI / test (pull_request) Failing after 1m30s
Details
CI / build-and-push (pull_request) Has been skipped
Details
CI / deploy-dev (pull_request) Has been skipped
Details
CI / deploy-uat (pull_request) Has been skipped
Details
41a887a73b 
- Add hasattr(sd, 'expression') guard in engine fixtures to prevent
  AttributeError when iterating over server_default columns that use
  DefaultClause (which lacks .expression)
- Add _build_engine_kwargs() in database.py to conditionally apply
  pool_size/max_overflow only for non-SQLite database URLs
- Fixes test failures in conftest.py, test_encrypted_json.py

Co-Authored-By: Paperclip <noreply@paperclip.ing>
Fix SQLite CI test failures: UUID binding, func.now() defaults, F402 lint
CI / lint (pull_request) Failing after 4s
Details
CI / typecheck (pull_request) Failing after 28s
Details
CI / test (pull_request) Failing after 2m37s
Details
CI / build-and-push (pull_request) Has been skipped
Details
CI / deploy-dev (pull_request) Has been skipped
Details
CI / deploy-uat (pull_request) Has been skipped
Details
9fbab62717 
Three fixes from PR #35 review:

1. Fix F402: rename loop var 'table' → 'metadata_table' in test_encrypted_json.py
2. Strip func.now() server_defaults in conftest.py engine/db_engine fixtures
3. Add aiosqlite UUID adapter for async engine

Model changes to provide Python-side defaults for SQLite compatibility:
- TimestampMixin: add default=_utcnow for created_at/updated_at
- UUIDPrimaryKeyMixin: use GuidType for cross-DB UUID handling
- User.id: use GuidType() instead of Text, Mapped[uuid.UUID]
- User.email_verified: add default=False
- Purchase.ingested_at: add default=_utcnow
- types.py: add GuidType TypeDecorator for UUID→String conversion

Fixes: CAR-1012
Checkout Charlie requested changes 2026-05-30 09:14:30 +00:00
Checkout Charlie left a comment
Member
Copy Link
Copy Source

QA Review — CAR-1012

Reviewed PR #40 against https://cartsnitch.dev.farh.net. The UUID/SQLite fix itself is solid — the model-layer tests pass. But CI is failing in three ways, all unrelated to the SQLite/UUID fix:

CI Failures

Job Status Root Cause
lint FAILING Ruff format check fails — trailing whitespace, line length, or similar formatting issue in one of the 7 changed files
typecheck FAILING 9 mypy errors in config.py, cache.py, rate_limit.py — all pre-existing, existed before this PR
test FAILING 116 failed / 55 passed — ALL 116 failures are 404 Not Found because routes are mounted at /api/v1 but tests hit /stores, /products, etc. without the prefix. Pre-existing routing configuration issue, not caused by this PR

What IS working

  • test_encrypted_json.py: all 6 tests pass (UUID binding works)
  • test_models.py: all 17 tests pass (server_default stripping works)
  • The SQLite UUID adapter (GuidType) and func.now() Python-side defaults are correctly implemented

Action Required

Request changes — the lint failure must be fixed before merge. Format the code with ruff format and ensure it passes.

The typecheck and test failures are pre-existing and should be tracked in separate issues; they should not block this PR from merging once lint is fixed.

Screenshot: test job showing 116× 404 failures (pre-existing routing mismatch)

## QA Review — CAR-1012 Reviewed PR #40 against https://cartsnitch.dev.farh.net. The UUID/SQLite fix itself is solid — the model-layer tests pass. But CI is failing in three ways, all unrelated to the SQLite/UUID fix: ### CI Failures | Job | Status | Root Cause | |-----|--------|------------| | lint | **FAILING** | Ruff format check fails — trailing whitespace, line length, or similar formatting issue in one of the 7 changed files | | typecheck | **FAILING** | 9 mypy errors in `config.py`, `cache.py`, `rate_limit.py` — all pre-existing, existed before this PR | | test | **FAILING** | 116 failed / 55 passed — ALL 116 failures are `404 Not Found` because routes are mounted at `/api/v1` but tests hit `/stores`, `/products`, etc. without the prefix. Pre-existing routing configuration issue, not caused by this PR | ### What IS working - `test_encrypted_json.py`: all 6 tests pass (UUID binding works) - `test_models.py`: all 17 tests pass (server_default stripping works) - The SQLite UUID adapter (`GuidType`) and `func.now()` Python-side defaults are correctly implemented ### Action Required **Request changes** — the lint failure must be fixed before merge. Format the code with `ruff format` and ensure it passes. The typecheck and test failures are pre-existing and should be tracked in separate issues; they should not block this PR from merging once lint is fixed. --- *Screenshot: test job showing 116× 404 failures (pre-existing routing mismatch)*
Barcode Betty closed this pull request 2026-06-06 02:11:43 +00:00
Some checks are pending
CI / lint (pull_request) Failing after 4s
Details
CI / typecheck (pull_request) Failing after 28s
Details
CI / test (pull_request) Failing after 2m37s
Details
CI / build-and-push (pull_request) Has been skipped
Details
CI / deploy-dev (pull_request) Has been skipped
Details
CI / deploy-uat (pull_request) Has been skipped
Details

Pull request closed

Please reopen this pull request to perform a merge.
Sign in to join this conversation.
Reviewers
No Reviewers
Checkout Charlie
Labels
Clear labels
bug
Something isn't working
 documentation
Improvements or additions to documentation
 duplicate
This issue or pull request already exists
 enhancement
New feature or request
 good first issue
Good for newcomers
 help wanted
Extra attention is needed
 invalid
This doesn't seem right
 question
Further information is requested
 wontfix
This will not be worked on
No Label
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
ai-review (AI Review) cs_betty (Barcode Betty) cs_charlie (Checkout Charlie) cpfarhood (Chris Farhood) ci (Continuous Integration [bot]) cs_carl (Coupon Carl) cs_dottie (Deal Dottie) flux (Flux CD) admin (Gitea Admin) cs_martha (Markdown Martha) renovate (Mend Renovate) cs_savannah (Savannah Savings) cs_steve (Stockboy Steve)
No Assignees
2 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: cartsnitch/api#40
Delete Branch "betty/fix-ci-test-failures-v2"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?