Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

forbid extra fields in BaseModel #44306

Open
wants to merge 9 commits into
base: main
Choose a base branch
from
Open

forbid extra fields in BaseModel #44306

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
3e4bfee
forbids extra fields
rawwar Nov 23, 2024
9d36d40
Merge branch 'main' of https://github.com/apache/airflow into kalyan/…
rawwar Nov 27, 2024
81fff16
Merge branch 'main' of https://github.com/apache/airflow into kalyan/…
rawwar Nov 27, 2024
565f71c
remove file_token before validation in DagResponse
rawwar Nov 27, 2024
b31c1af
remove extra fields from PluginResponse
rawwar Nov 27, 2024
c2c006c
add priority_weight_strategies
rawwar Nov 27, 2024
51d94e3
fix dag_run test
rawwar Nov 27, 2024
ec89eec
update test_should_200_for_unknown_fields
rawwar Nov 27, 2024
4b8806a
Small improvement
pierrejeambrun Nov 29, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion airflow/api_fastapi/core_api/base.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,4 +26,4 @@ class BaseModel(PydanticBaseModel):
:meta private:
"""

model_config = ConfigDict(from_attributes=True)
model_config = ConfigDict(from_attributes=True, extra="forbid")
8 changes: 8 additions & 0 deletions airflow/api_fastapi/core_api/datamodels/dags.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
ConfigDict,
computed_field,
field_validator,
model_validator,
)

from airflow.api_fastapi.core_api.base import BaseModel
Expand Down Expand Up @@ -91,6 +92,13 @@ def file_token(self) -> str:
serializer = URLSafeSerializer(conf.get_mandatory_value("webserver", "secret_key"))
return serializer.dumps(self.fileloc)

@model_validator(mode="before")
@classmethod
def remove_file_token(cls, data):
rawwar marked this conversation as resolved.
Show resolved Hide resolved
if isinstance(data, dict):
data.pop("file_token", None)
return data


class DAGPatchBody(BaseModel):
"""Dag Serializer for updatable bodies."""
Expand Down
3 changes: 3 additions & 0 deletions airflow/api_fastapi/core_api/datamodels/plugins.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,9 @@ class PluginResponse(BaseModel):
ti_deps: list[Annotated[str, BeforeValidator(coerce_to_string)]]
listeners: list[str]
timetables: list[str]
priority_weight_strategies: list[Any]
admin_views: list[Any]
menu_links: list[Any]
Comment on lines +78 to +80
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure what types to add here.

I tried to add list[type[PriorityWeightStrategy]] to priority_weight_strategies. But, its throwing following error when generating openapi spec.

raceback (most recent call last):
  File "/opt/airflow/scripts/in_container/run_update_fastapi_api_spec.py", line 38, in <module>
    get_openapi(
  File "/usr/local/lib/python3.9/site-packages/fastapi/openapi/utils.py", line 493, in get_openapi
    field_mapping, definitions = get_definitions(
  File "/usr/local/lib/python3.9/site-packages/fastapi/_compat.py", line 231, in get_definitions
    field_mapping, definitions = schema_generator.generate_definitions(
  File "/usr/local/lib/python3.9/site-packages/pydantic/json_schema.py", line 361, in generate_definitions
    self.generate_inner(schema)
  File "/usr/local/lib/python3.9/site-packages/pydantic/json_schema.py", line 441, in generate_inner
    if 'ref' in schema:
  File "/usr/local/lib/python3.9/_collections_abc.py", line 769, in __contains__
    self[key]
  File "/usr/local/lib/python3.9/site-packages/pydantic/_internal/_mock_val_ser.py", line 41, in __getitem__
    return self._get_built().__getitem__(key)
  File "/usr/local/lib/python3.9/site-packages/pydantic/_internal/_mock_val_ser.py", line 58, in _get_built
    raise PydanticUserError(self._error_message, code=self._code)
pydantic.errors.PydanticUserError: `TypeAdapter[typing.Annotated[airflow.api_fastapi.core_api.datamodels.plugins.PluginCollectionResponse, FieldInfo(annotation=PluginCollectionResponse, required=True)]]` is not fully defined; you should define `typing.Annotated[airflow.api_fastapi.core_api.datamodels.plugins.PluginCollectionResponse, FieldInfo(annotation=PluginCollectionResponse, required=True)]` and all referenced types, then call `.rebuild()` on the instance.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes I think Generic inside Generic are not well supported T[K[V]]

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This new fields shouldn't be returned by the API, they are not needed at the moment.

Copy link
Member

@pierrejeambrun pierrejeambrun Nov 29, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can either pop them manually before using the model (not great because we would need to extend that to every endpoint constructing a XXXXResponse object that has a richer dict as a source), or allow extra for XXXXResponse and only apply it to XXXXBody


@field_validator("source", mode="before")
@classmethod
Expand Down
Loading
Loading