Migration to Pydantic v2: Enable compatibility with later FastAPI versions - #5017
Conversation
Co-authored-by: marrobi <17089773+marrobi@users.noreply.github.com>
…ility layer Co-authored-by: marrobi <17089773+marrobi@users.noreply.github.com>
…grate .dict() to .model_dump() Co-authored-by: marrobi <17089773+marrobi@users.noreply.github.com>
Co-authored-by: marrobi <17089773+marrobi@users.noreply.github.com>
…n, and .dict() calls Co-authored-by: marrobi <17089773+marrobi@users.noreply.github.com>
- Remove all try/except blocks providing Pydantic v1 fallback support - Update imports to use only Pydantic v2 (TypeAdapter instead of parse_obj_as) - Clean up TypeAdapter usage throughout codebase - Fix syntax errors and whitespace issues - Maintain all existing functionality with Pydantic v2 patterns Co-authored-by: marrobi <17089773+marrobi@users.noreply.github.com>
….8.6->0.9.0 Co-authored-by: marrobi <17089773+marrobi@users.noreply.github.com>
Co-authored-by: marrobi <17089773+marrobi@users.noreply.github.com>
…paces.py Co-authored-by: marrobi <17089773+marrobi@users.noreply.github.com>
… compatability with v1
- add explicit defaults for nullable model fields - restore User-to-dict conversion for persisted resource actors - migrate remaining serialization to model_dump() - fix nested Event Grid payload serialization - restore removed resource history behavior and route imports - preserve legacy role ID, optional email, and cost date handling - update tests for Pydantic v2 response types and error messages
…tedResource classes
Good catch from copilot, we can assume that v1 and v2 will not be installed at the same time and that for imports - this is superfluous Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 121 out of 121 changed files in this pull request and generated 1 comment.
Suppressed comments (1)
templates/shared_services/certs/template_schema.json:26
- This changes the certs bundle schema, but
templates/shared_services/certs/porter.yamlremains at 0.7.11. Without a patch version bump, registries and deployments can continue resolving the old bundle despite the schema change. Bump the certs bundle version as done for the other modified templates in this PR.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 121 out of 121 changed files in this pull request and generated 2 comments.
Suppressed comments (3)
templates/shared_services/certs/template_schema.json:26
- This bundle's schema changes, but unlike the other modified bundles its
porter.yamlversion remains at 0.7.11. Bundle changes must receive a semantic version bump so the updated schema can be published and selected; increment the certs bundle version as part of this PR.
api_app/models/schemas/airlock_request.py:86 - Making
typerequired changes the public request contract: Pydantic v1 accepted payloads without this field via the empty-string default, while this model now returns HTTP 422 (as the new regression test confirms). That contradicts the PR's “zero breaking changes” claim; either preserve compatibility or explicitly treat and version this as a breaking API change.
api_app/models/schemas/airlock_request.py:100 - This also introduces a breaking request-contract change: under v1, omitting
approvalused the falsey""default and produced a rejected review, whereas the migrated endpoint now returns HTTP 422. Preserve the prior behavior or document and version the review endpoint change as breaking rather than claiming zero API changes.
Co-authored-by: ChrisChapman-gh <118748128+ChrisChapman-gh@users.noreply.github.com>
|
/test-extended |
|
🤖 pr-bot 🤖 🏃 Running extended tests: https://github.com/microsoft/AzureTRE/actions/runs/30997933316 (with refid (in response to this comment from Chris Chapman (@ChrisChapman-gh)) |
…able types so validations are clean
…ints on non-nullable properties and updating template normalization logic - app was giving 400 errors and UI would fail when submitting.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 124 out of 125 changed files in this pull request and generated no new comments.
Files not reviewed (1)
- ui/app/package-lock.json: Generated file
Suppressed comments (4)
api_app/models/domain/airlock_request.py:108
- Legacy records created when
AirlockRequestInCreate.typewas omitted contain"type": "", because the v1 default was serialized into Cosmos. Making this field optional only handles missing/null values;TypeAdapter(AirlockRequest)still rejects the persisted empty string, so list/get operations can still fail despite the stated backward-compatibility guarantee. Add a pre-validator/data migration for the empty-string representation as well (and define how such records can be used whereevent_sender.py:17requires a real type).
type: Optional[AirlockRequestType] = Field(None, title="Airlock request type")
api_app/models/schemas/airlock_request.py:86
- This changes the request contract from an optional field with a default to a required field, so existing API clients that omit
typenow receive 422. That contradicts the PR's “zero breaking changes” claim. Either preserve a compatible default/legacy handling or explicitly treat and version this as a breaking API change.
api_app/models/schemas/airlock_request.py:100 - This likewise makes
approvalrequired, whereas the v1 model accepted an omitted value and the review flow treated its falsey default as rejection. Existing clients can therefore change from a successful request to 422, contrary to the stated no-breaking-change guarantee. Preserve the previous behavior (for example, an explicitFalsedefault) or document/version the contract break.
templates/shared_services/certs/template_schema.json:31 - The certs bundle schema is modified, but
templates/shared_services/certs/porter.yamlremains at 0.7.11. Bundle changes require a semantic version increment; without it this updated schema will not be published/upgraded as a new component version. Bump the certs Porter version (and reflect it in release metadata as applicable), as done for the other changed bundles in this PR.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 124 out of 125 changed files in this pull request and generated 1 comment.
Files not reviewed (1)
- ui/app/package-lock.json: Generated file
Suppressed comments (1)
templates/shared_services/certs/template_schema.json:26
- This changes the certs bundle schema, but
templates/shared_services/certs/porter.yamlremains at0.7.11. Bundle changes must increment the Porter version so the updated schema can be published and distinguished from the existing artifact; bump it to the next patch version as done for the other modified templates in this PR.
|
/test-extended |
|
🤖 pr-bot 🤖 🏃 Running extended tests: https://github.com/microsoft/AzureTRE/actions/runs/31195708239 (with refid (in response to this comment from Chris Chapman (@ChrisChapman-gh)) |
…sions (microsoft#5017) * Initial plan * Implement Pydantic v1/v2 compatibility layer and update requirements Co-authored-by: marrobi <17089773+marrobi@users.noreply.github.com> * Complete Pydantic v2 migration for airlock processor and add compatibility layer Co-authored-by: marrobi <17089773+marrobi@users.noreply.github.com> * Fix Pydantic v2 compatibility issues: update field annotations and migrate .dict() to .model_dump() Co-authored-by: marrobi <17089773+marrobi@users.noreply.github.com> * Complete Pydantic v2 migration: fix parse_obj_as usage and test fixtures Co-authored-by: marrobi <17089773+marrobi@users.noreply.github.com> * Fix remaining Pydantic v2 issues: TypeAdapter imports, user validation, and .dict() calls Co-authored-by: marrobi <17089773+marrobi@users.noreply.github.com> * Remove Pydantic v1 backward compatibility and fix linting issues - Remove all try/except blocks providing Pydantic v1 fallback support - Update imports to use only Pydantic v2 (TypeAdapter instead of parse_obj_as) - Clean up TypeAdapter usage throughout codebase - Fix syntax errors and whitespace issues - Maintain all existing functionality with Pydantic v2 patterns Co-authored-by: marrobi <17089773+marrobi@users.noreply.github.com> * Increment component versions: API 0.24.5->0.25.0, Airlock Processor 0.8.6->0.9.0 Co-authored-by: marrobi <17089773+marrobi@users.noreply.github.com> * Fix linting issues: remove unused imports from template route files Co-authored-by: marrobi <17089773+marrobi@users.noreply.github.com> * Fix unit test errors: remove double .model_dump() calls in test_workspaces.py Co-authored-by: marrobi <17089773+marrobi@users.noreply.github.com> * WIP * Remove some of the compatibility code. * Switch to User objects * Simplify serialization. * Add tests back * update tests * Updates to simplify. * Updates to simplify code. * Fix user model * Update models to correct syntax for v2 * Address comments. * Update pydantic version and refactor user fields to maintain backward compatability with v1 * Remove leftover merge marker from changelog * Fix Pydantic v2 compatibility regressions - add explicit defaults for nullable model fields - restore User-to-dict conversion for persisted resource actors - migrate remaining serialization to model_dump() - fix nested Event Grid payload serialization - restore removed resource history behavior and route imports - preserve legacy role ID, optional email, and cost date handling - update tests for Pydantic v2 response types and error messages * Bump version to 0.8.12 for airlock_processor and 0.25.28 for api_app * Remove unnecessary newline at the end of ResourceTemplate and RestrictedResource classes * Apply suggestions from code review Good catch from copilot, we can assume that v1 and v2 will not be installed at the same time and that for imports - this is superfluous Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> * - Refactor resource models to use default_factory for list and dict fields as per copilot recomendations. - Fix indentation * Refactor schema fields to use default_factory for list and dict types * Apply suggestions from code review Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> * Refactor response models to support union types and update default values for dict fields using default_factory * Refactor user field in Operation and Resource models to use default_factory for dict types * Refactor validation logic in AirlockRequestRepository and ResourceRepository to remove Pydantic version checks; update test to use actual user ID. * update to fix miss handeling of model_dump * add handelling for null values * Fix OpenAPI schema generation under Pydantic v2 Serialize Property model instances in template response examples to plain dicts so they don't leak into json_schema_extra and break schema generation (TypeError: unhashable type: 'Property'). Add a regression test that renders the full OpenAPI schema. * Refactor AirlockRequest and AirlockReview models to remove default values for decision fields; update tests to include request type in mock responses. * Apply suggestions from code review Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> * bump api version after it was reset during a rebase * Refactor AuthenticatedUser model to use ConfigDict for immutability; update tests for role mutation checks and enhance schema service tests for recursive null property removal. * Fix template schema validation errors and bump affected bundle versions The API started failing workspace service creation with SchemaError after dependency upgrades. Root cause was legacy property-level JSON Schema $id fragments (for example "#/properties/..."), which are rejected by newer jsonschema metaschema validation. Changes: - Removed legacy nested property-level $id entries from updated template_schema.json files. - Added defensive schema normalization in API validation to strip invalid nested $id fragments before jsonschema.validate, while preserving top-level schema metadata. - Added a regression test to ensure legacy nested $id values no longer cause SchemaError. - Bumped porter bundle versions (patch increments) for every template whose template_schema.json was modified, including shared services, workspace services, and guacamole user resource bundles. Why: - Restore reliability of template input validation for create/update flows. - Keep compatibility with already-registered/legacy template content during rollout. - Keep template bundle versioning consistent with schema changes so upgrades are traceable. * Handle user model serialization gracefully in resource update * [resources.py (line 50)](/workspaces/AzureTRE/api_app/db/repositories/resources.py:50) now removes any nested $id with a non-empty URI fragment, including #/properties/..., #properties/..., and absolute URI fragments. Root $id, valid nested IDs, and the original schema object remain unchanged. [test_resource_repository.py (line 413)](/workspaces/AzureTRE/api_app/tests_ma/test_db/test_repositories/test_resource_repository.py:413) covers all three invalid forms, including the exact firewall value. * fix: restore ResourceTemplate.properties type to Dict[str, Property] properties was changed from Dict[str, Property] to Dict[str, Any] to fix a jsonschema.SchemaError caused by the legacy Property model serialising optional fields as null (e.g. "items": null), which is invalid in JSON Schema. Three changes make Dict[str, Property] safe again: - Property.model_config adds extra="allow" so unknown JSON Schema keywords ($ref, oneOf, format, if/then/else, etc.) are preserved rather than silently dropped on deserialisation. - Property type field is made Optional[str] so properties that use $ref or const without an explicit type are accepted. - Property gains a @model_serializer(mode='plain') that emits only explicitly- set fields (model_fields_set), excludes None values, and recurses into nested plain-dict sub-schemas (items, properties) to strip any legacy null values. A hasattr guard handles the edge case where Pydantic calls the serialiser with an uncoerced plain dict due to item-level dict assignment bypassing validate_assignment. ResourceTemplate gains validate_assignment=True so direct field assignment coerces dict values to Property instances, and a @model_serializer(mode='wrap') that calls _strip_none_recursive on the full serialised output to cover allOf and other plain-dict fields that Pydantic's exclude_none does not recurse into. The legacy remove_legacy_null_property_fields function and its LEGACY_NULL_PROPERTY_FIELDS allowlist in schema_service are removed; null sanitisation is now owned by the model layer. The test was reproducing that step to put the mock enriched_template_mock.return_value into the same state it would be in after enrich_template had run. Now that ResourceTemplate._serialize calls _strip_none_recursive, allOf: None is stripped during model_dump() itself — so neither the guard in enrich_template nor the pop in the test is needed. The pop("allOf", None) is now a no-op and can be removed entirely * Fix, PR comments and feedback * fix: update AirlockRequest and Operation models to use Optional types and set default status * fix defaults * update schema examples to present correct types. * fix: set default value for previous_status in RequestProperties and update enum type in Property model * Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> * Updated [resource.py (line 113)](/workspaces/AzureTRE/api_app/models/domain/resource.py:113) so value is required while explicit JSON null remains valid: * Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> * Restore legacy airlock request type compatibility Co-authored-by: ChrisChapman-gh <118748128+ChrisChapman-gh@users.noreply.github.com> * enhance schema normalization to ignore legacy const:null for non-nullable types so validations are clean * Enhance resource validation by removing accidental const:null constraints on non-nullable properties and updating template normalization logic - app was giving 400 errors and UI would fail when submitting. * revert servicebus update * remove unused def --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: marrobi <17089773+marrobi@users.noreply.github.com> Co-authored-by: Marcus Robinson <marrobi@microsoft.com> Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
This PR migrates the Azure TRE codebase from Pydantic v1.10.19 to v2.13.4 to enable compatibility with later versions of FastAPI that require Pydantic v2.
Overview
Later versions of FastAPI require Pydantic v2, and this migration ensures Azure TRE can upgrade FastAPI without being blocked by Pydantic version constraints.
Key Changes
🔧 Core Infrastructure Updates
api_app/requirements.txtandairlock_processor/requirements.txtnow specify Pydantic v2.13.4🏗️ Model Architecture Migration
AzureTREModelnow uses Pydantic v2ConfigDictwith v1 fallbackallow_population_by_field_name→populate_by_name@validatorto@field_validatorwith compatibility layer📦 Component Updates
parse_obj_as→TypeAdapterpatternbump-pydantictoolExample Migration Pattern
Before (Pydantic v1):
After (Pydantic v2 with v1 compatibility):
Testing & Validation
✅ Comprehensive test suite: All existing functionality preserved
✅ FastAPI compatibility: Confirmed working with FastAPI 0.115.3
✅ Component isolation: API app and airlock processor independently validated
✅ Migration tools: Used official
bump-pydantictool for schema updatesImpact
Migration Benefits
Fixes #4637.
💬 Share your feedback on Copilot coding agent for the chance to win a $200 gift card! Click here to start the survey.