fix: CRUD write paths, per-viewset body schemas, 409 responses, packaging (v1.5.2) - #13
Merged
Merged
Conversation
- register(): clone handlers before patching body annotations — viewset schemas no longer leak across instances (broke multi-viewset apps) - PATCH validated against auto-generated all-optional <Schema>Patch model; explicit JSON null clears nullable columns - PUT writes only payload fields, never nulls columns absent from the schema and never touches the primary key - adapters: strip NULL primary key on create (fixes Tortoise POST and PostgreSQL inserts with Optional id schemas) - integrity violations return 409 with sanitized message; raw SQL no longer exposed in response bodies - list(): Query(ge=0, le=10000) validation for limit/offset - LIST filters from ListConfig advertised in OpenAPI via openapi_extra - packaging: SQLAlchemy/uvicorn removed from core deps; sqlalchemy extra installs SQLAlchemy[asyncio] (includes greenlet) - tests: 9 new regression tests; updated PUT/PATCH/409 semantics tests - docs: README/docs/release notes for 1.5.2
The base install became ORM-agnostic (SQLAlchemy moved to the 'sqlalchemy' extra), but tests/conftest.py imports sqlalchemy unconditionally. The CI matrix installs '.[test]' and the test-extras job installs '.[test,tortoise,peewee]' — neither pulls SQLAlchemy, so every test job died with ModuleNotFoundError: No module named 'sqlalchemy'. Add SQLAlchemy[asyncio]>=2.0.0 to the test extra (dev-only; the published wheel stays ORM-agnostic).
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
* Add regression tests for the _clone_bound_method fallback branch (handlers that are not plain bound methods, incl. unpatchable ones) and for the Query-default fallback in sync/async list() when called programmatically. * The test-extras job (tortoise/peewee adapters installed) now runs coverage and uploads it to codecov with the 'extras' flag, so the adapter lines changed in this PR count towards patch coverage — previously only the 3.11 matrix job (no ORM extras) uploaded.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Found by running a test project against the PyPI 1.5.1 build across SQLAlchemy sync/async, Peewee and Tortoise.
Critical fixes
register()mutated__annotations__of the shared class-level CRUD functions, so with 2+ viewsets every POST/PUT/PATCH endpoint validated against the last registered schema. Handlers are now cloned per instance before annotation patching. Regression test: two viewsets, one process.exclude_unsetcould help. PATCH now validates against an auto-generated all-optional<Schema>Patchmodel.null— JSON null now clears nullable columns.id: Optional[int] = Noneschema pattern (id is non nullable field, but null was passed) — adapters strip NULL primary keys on create (also fixes latent PostgreSQL issue).409 Conflictwith a sanitized message across all three adapters.Minor fixes
limit/offsetvalidated (ge=0,limit<=10000); negative values → 422 (previouslylimit=-5returned all rows).ListConfigfilters and__opvariants advertised in OpenAPI (openapi_extra).SQLAlchemy/uvicornremoved from core deps; package imports without SQLAlchemy.[sqlalchemy]extra installsSQLAlchemy[asyncio](includesgreenlet— async worked out of the box previously only by accident).Tests & docs
tests/test_write_path_regressions.py); existing PUT/PATCH/integrity tests updated to the new semantics.RELEASE_1.5.2.mdadded; version bumped to 1.5.2.Behavior changes (documented in RELEASE_1.5.2.md)
nullwrites NULL.pip install fastapi-viewsetsno longer pulls SQLAlchemy/uvicorn.Note:
ruff checkreports many pre-existing issues repo-wide (1700+); this PR follows the existing code style and does not reformat unrelated code.