feat: migrate to Pydantic v2 #3862
Conversation
BREAKING CHANGE: Requires pydantic>=2.0 - Update pydantic dependency from v1 to v2 - Remove v1/v2 compatibility shim in samtranslator/compat.py - Migrate deprecated APIs: - parse_obj() -> model_validate() - schema() -> model_json_schema() - dict() -> model_dump() - __root__ -> RootModel with .root accessor - class Config -> model_config = ConfigDict() - Update Field() to use json_schema_extra for custom schema properties - Add explicit default values for all Optional fields - Add type aliases to avoid field name shadowing in Pydantic v2 - Update JSON schema generation to normalize $defs -> definitions - Upgrade schema version from draft-04 to draft-07 - Update validation error handling for v2 error format - Update mypy to >=1.5.0 for Pydantic v2 plugin compatibility - Add hypothesis test dependency
bnusunny
force-pushed
the
pydantic_v2_upgrade
branch
from
ca14894 to
761f208
Compare
| pytest>=6.2,<8 | ||
| parameterized~=0.7 | ||
| parameterized>=0.9,<1 | ||
| hypothesis>=6.0,<7 |
There was a problem hiding this comment.
what is this new dependency?
|
|
||
| # CloudFormation CLI tools | ||
| cloudformation-cli>=0.2.39,<0.3.0 | ||
| # cloudformation-cli>=0.2.39,<0.3.0 |
There was a problem hiding this comment.
I already deleted this in the newest version of the code
| # Note: Using ... (Ellipsis) to make this field required in Pydantic v2 | ||
| SubnetIds: List[SamIntrinsicable[str]] |
There was a problem hiding this comment.
I don't know what this comment is about. I don't see any ellipsis
| # Required fields - not using get_prop() to ensure they are truly required in Pydantic v2 | ||
| Source: ResourceReference | ||
| Destination: Union[ResourceReference, List[ResourceReference]] | ||
| Permissions: List[Literal["Read", "Write"]] |
There was a problem hiding this comment.
We are the ones making it non required by adding default=None in samtranslator/internal/schema_source/common.py line 88.. But also by not using get_prop we effectively remove the docs too. So we probably need maybe a second version of get_prop that doesn't define a default... or an extra parameter for the function returned by get_prop to get the different cases.
There was a problem hiding this comment.
Actually, I see that passthrough_prop has a required parameter now, so we should be able to do something similar to that.
| # Pydantic v1.10.17+ | ||
| import pydantic # type: ignore | ||
| # Pydantic v2 direct import - no compatibility shim needed | ||
| import pydantic |
There was a problem hiding this comment.
Do we even use this file anymore? It looks like we replaced it everywhere (or almost?).
| - tests/translator/output/**/function_with_tenancy_and_api_event.json # cfnlint is not updated to recognize the TenancyConfig property | ||
| - tests/translator/output/**/function_with_tenancy_and_httpapi_event.json # cfnlint is not updated to recognize the TenancyConfig property | ||
| - tests/translator/output/**/function_with_tenancy_config_global.json # cfnlint is not updated to recognize the TenancyConfig property | ||
| - tests/translator/output/**/*durable_config*.json # TODO: Remove this once Durable Function is launched in CFN |
There was a problem hiding this comment.
All of these in this file were removed in a previous PR: https://github.com/aws/serverless-application-model/blob/761f208bcdb25ec25d0700bf0e1e68f2a2b589b1/.cfnlintrc.yaml and this is bringing them back (we shouldn't)
| mypy --strict samtranslator bin schema_source | ||
| # cfn-lint to make sure generated CloudFormation makes sense | ||
| bin/run_cfn_lint.sh | ||
| # bin/run_cfn_lint.sh |
There was a problem hiding this comment.
I imagine we don't want to comment out this in the final version
2 participants
migrate to Pydantic v2
Checklist
Examples?
Please reach out in the comments if you want to add an example. Examples will be
added to
sam initthrough aws/aws-sam-cli-app-templates.By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.