kaxil commented on code in PR #63716:
URL: https://github.com/apache/airflow/pull/63716#discussion_r2940827928
##########
airflow-core/src/airflow/api_fastapi/core_api/base.py:
##########
@@ -54,12 +54,12 @@ def make_partial_model(model: type[PydanticBaseModel]) ->
type[PydanticBaseModel
"""Create a version of a Pydantic model where all fields are Optional with
default=None."""
field_overrides: dict = {}
for field_name, field_info in model.model_fields.items():
- new_info = deepcopy(field_info)
ann = field_info.annotation
origin = get_origin(ann)
if not (origin is Union and type(None) in get_args(ann)):
ann = ann | None # type: ignore[operator, assignment]
- new_info.default = None
+ new_info = FieldInfo(default=None)
Review Comment:
Constructing a fresh `FieldInfo(default=None)` and only copying `.metadata`
drops every other FieldInfo attribute from the original field: `alias`,
`validation_alias`, `serialization_alias`, `alias_priority`, `title`,
`description`, `json_schema_extra`, `repr`, `frozen`, `deprecated`, `exclude`,
`discriminator`, and `examples`.
This breaks real models. `ConnectionBody.connection_id` has
`serialization_alias='conn_id'`, `ConnectionBody.schema_` has `alias='schema'`,
and `VariableBody.value` has `serialization_alias='val'`. After this change,
those partial models lose all their aliases. With `extra='forbid'` on
`ConnectionBody`, a PATCH sending `{"schema": "public"}` would get rejected as
"extra inputs not permitted" because the `schema` validation alias on `schema_`
is gone.
I verified this on pydantic 2.12.5 (the installed version). The old
`deepcopy` approach works fine there too, so I'm not sure what breakage this is
fixing. If there is a specific pydantic version or scenario where `deepcopy`
fails, could you share it? The fix would need to preserve all field attributes,
not just metadata.
##########
airflow-core/src/airflow/api_fastapi/core_api/base.py:
##########
@@ -54,12 +54,12 @@ def make_partial_model(model: type[PydanticBaseModel]) ->
type[PydanticBaseModel
"""Create a version of a Pydantic model where all fields are Optional with
default=None."""
field_overrides: dict = {}
for field_name, field_info in model.model_fields.items():
- new_info = deepcopy(field_info)
ann = field_info.annotation
origin = get_origin(ann)
if not (origin is Union and type(None) in get_args(ann)):
ann = ann | None # type: ignore[operator, assignment]
- new_info.default = None
+ new_info = FieldInfo(default=None)
+ new_info.metadata = list(field_info.metadata)
Review Comment:
The existing test suite doesn't catch this because `SampleModel` has no
aliased fields. Adding a field like `value: str =
Field(serialization_alias='val')` or `schema_: str | None = Field(None,
alias='schema')` to the test model and asserting the partial model preserves
those aliases would prevent this from slipping through.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
