fix: migrate deprecated framework contracts - #85
Conversation
|
Warning Review limit reachedNext included review available in 45 minutes. View limit detailsLimit details: You’ve used the included review currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Review configuration: ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (15)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Closed as superseded: the deprecation-free runtime migration directly overlapped #73 in |
Pull request was closed
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
There was a problem hiding this comment.
🔴 Datasets with no schema pass validation and publish
After the field rename, getattr(dataset, "schema", None) in validate_metadata resolves to Pydantic's deprecated schema method instead of the column list, because _REQUIRED_METADATA_FIELDS still holds the string "schema" (catalog.py:146). The method is always truthy, so a dataset with an empty schema is never flagged missing, and publish_dataset publishes it.
(Refers to this code)
Prompt for agents
The Dataset model field was renamed from `schema` to `dataset_schema` (with alias `schema`) in src/sdp_core/contracts.py. However `_REQUIRED_METADATA_FIELDS` in src/sdp/catalog.py (line 146) still lists the string "schema", and validate_metadata (line 364) checks presence via getattr(dataset, field, None). Since `schema` is no longer an attribute of the model, getattr now returns Pydantic's deprecated BaseModel.schema method, which is always truthy, so the schema requirement is never detected as missing. This lets datasets with an empty schema pass validate_metadata and be published. Fix validate_metadata so the schema requirement maps to the actual attribute `dataset_schema` (e.g. a name-mapping table {public_name: attr_name}) while keeping the public-facing `required_fields`/`missing` list using the external name "schema" for API compatibility.
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
📝 Info: Alias round-trip in patch_dataset is consistent
serialize_by_alias=True makes model_dump() emit schema, and patch_dataset both keys off "schema" in updates and rebuilds with model_validate; populate_by_name=True keeps the round-trip valid. The interaction is subtle and easy to break in later edits.
(Refers to this code)
Was this helpful? React with 👍 or 👎 to provide feedback.
| dev = [ | ||
| "pytest==9.1.1", | ||
| "httpx==0.28.1", | ||
| "httpx2==2.12.0", |
There was a problem hiding this comment.
🔴 Test suite broken by swapping httpx for httpx2
The dev extra now pins httpx2 instead of httpx. Starlette's TestClient does import httpx and raises if it is absent, and httpx2 does not provide the httpx module. Installing .[dev] no longer pulls httpx, so importing TestClient fails and every API test errors.
| "httpx2==2.12.0", | |
| "httpx==0.28.1", |
Was this helpful? React with 👍 or 👎 to provide feedback.
| dev = [ | ||
| "pytest==9.1.1", | ||
| "httpx==0.28.1", | ||
| "httpx2==2.12.0", |
There was a problem hiding this comment.
🔍 Requirements lockfiles not regenerated for dep change
requirements-dev.txt and requirements-test.txt still pin httpx==0.28.1 while the dev extra changed. CLAUDE.md requires regenerating these with uv pip compile --generate-hashes on any dependency change; they were not touched here.
Was this helpful? React with 👍 or 👎 to provide feedback.
1 participant
Root cause
The protected-main test/runtime boundary emitted four warning classes that were being tolerated instead of resolved:
httpx2forTestClient; the dev extra still installed legacyhttpx.schema, shadowing the deprecatedBaseModel.schemaAPI.on_eventhook.Change
httpx2==2.12.0and commit the reproducibleuv.lock;schemathrough Pydantic aliases while using the non-conflicting internaldataset_schema;No warning suppression is added. The external API continues accepting and emitting
schema.Verification
uv run pytest -q -W error::DeprecationWarning -W error::UserWarningResult: full suite passed, including Hypothesis properties; eight live-integration tests skipped by their existing runtime guard.
git diff --checkpassed.Authoritative references
Starlette. (2026). Test client. https://www.starlette.io/testclient/
Pydantic Services Inc. (2026). HTTPX2 2.12.0. https://pypi.org/project/httpx2/2.12.0/