kaxil commented on code in PR #65407:
URL: https://github.com/apache/airflow/pull/65407#discussion_r3153670086
##########
airflow-core/src/airflow/api_fastapi/core_api/datamodels/dags.py:
##########
@@ -44,6 +45,11 @@
from airflow.serialization.definitions.param import SerializedParamsDict
+def _is_response_safe_pod_override(value: Any) -> bool:
+ """Whether a pod_override value is already safe to preserve in the
response."""
+ return value is None or isinstance(value, str | int | float | Mapping |
list)
Review Comment:
`Mapping` and `list` are accepted shallowly here -- contents aren't walked.
So a `pod_override` like `{"sidecar": V1Pod(...)}` (or a list containing a
`V1Pod`) passes the check and re-triggers the original
`PydanticSerializationError` / 500 that this PR is meant to prevent.
In practice users typically pass a single `V1Pod`, so this is an edge case
rather than a blocker. But the docstring on `get_default_args` ("Only
`executor_config['pod_override']` values outside the JSON-primitive
allow-list... are rewritten") overstates the guarantee -- it's only safe when
the container's contents are also JSON primitives. Either recurse the check
(treat any nested non-primitive as unsafe), or soften the wording so future
readers don't rely on it as a hard guarantee.
--
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]
