Upgrade to pydantic v2 #141
Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
There was a problem hiding this comment.
CodeQL found more than 20 potential problems in the proposed changes. Check the Files changed tab for more details.
pycfmodel/model/generic.py
Outdated
| ResolvableDateOrList, | ||
| ResolvableDatetimeOrList, |
There was a problem hiding this comment.
Changes the order due to the new lax config of pydantic. Date can be parsed inside a Datetime
There was a problem hiding this comment.
It might be a good idea to add a comment mentioning this as it won't be obvious going forward
| @@ -23,12 +23,6 @@ class Resource(CustomModel): | |||
| UpdatePolicy: Optional[Dict] = None | |||
| UpdateReplacePolicy: Optional[ResolvableStr] = None | |||
|
|
|||
| @validator("Type") | |||
There was a problem hiding this comment.
This has been replaced by a Discriminated Union
|
|
||
|
|
||
| class GenericResource(Resource): | ||
| """This class is used for all resource types that don't have a dedicated class.""" | ||
|
|
||
| ALLOW_EXISTING_TYPES: ClassVar[bool] = True | ||
| Type: str | ||
| Type: Optional[str] = None # Optional to handle cases like "AWS::CloudFormation::Authentication" |
There was a problem hiding this comment.
@ignaciobolonio Review this case
There was a problem hiding this comment.
I don't have any issue with this as long as it has retrocompatibility
There was a problem hiding this comment.
There are a LOT of changes but it follows a similar way to my attempt here https://github.com/Skyscanner/pycfmodel/tree/update_pydantic_v2 It is amazing work Oscar, great contribution
|
|
||
|
|
||
| class GenericResource(Resource): | ||
| """This class is used for all resource types that don't have a dedicated class.""" | ||
|
|
||
| ALLOW_EXISTING_TYPES: ClassVar[bool] = True | ||
| Type: str | ||
| Type: Optional[str] = None # Optional to handle cases like "AWS::CloudFormation::Authentication" |
There was a problem hiding this comment.
I don't have any issue with this as long as it has retrocompatibility
pycfmodel/action_expander.py
Outdated
| @@ -34,9 +34,9 @@ def _expand_actions(actions: Union[str, List[str]], not_action=False) -> List[st | |||
| def expand_actions(obj): | |||
| if isinstance(obj, dict): | |||
| for key, value in obj.items(): | |||
| if key == "Action": | |||
| if key == "Action" and value is not None: | |||
There was a problem hiding this comment.
| if key == "Action" and value is not None: | |
| if value is None: | |
| continue # or obj[key] = None | |
| elif key == "Action": |
Otherwise we'll end up recursively calling expand actions. I'm not sure if that's what we want, but in any case it should change the current behaviour.
|
|
||
| from pycfmodel.model.base import FunctionDict | ||
|
|
||
|
|
||
| class LooseIPv4Network(_BaseNetwork): | ||
| class LooseIPv4Network: | ||
| __slots__ = () |
There was a problem hiding this comment.
It's to make it more efficient, we don't want to store anything in memory
pycfmodel/model/types.py
Outdated
| @@ -78,15 +135,19 @@ def validate(cls, value: Any) -> bool: | |||
|
|
|||
| ResolvableIPv4Network = Resolvable[LooseIPv4Network] | |||
| ResolvableIPv6Network = Resolvable[LooseIPv6Network] | |||
| ResolvableIPvXNetwork = Annotated[ | |||
There was a problem hiding this comment.
| ResolvableIPvXNetwork = Annotated[ | |
| ResolvableIPNetwork = Annotated[ |
setup.py
Outdated
| @@ -28,7 +28,7 @@ | |||
|
|
|||
| setup( | |||
| name="pycfmodel", | |||
| version="0.22.0", | |||
| version="0.23.0", | |||
There was a problem hiding this comment.
agree, and updated to 1.0.0
* Add boolean coercion to string * Minor fixes * Minor fixes * Update cf actions
Co-authored-by: Jordi Soucheiron <[email protected]>
No description provided.