jason810496 commented on code in PR #54597:
URL: https://github.com/apache/airflow/pull/54597#discussion_r2303056803
##########
airflow-core/src/airflow/api_fastapi/core_api/routes/public/variables.py:
##########
@@ -141,26 +144,18 @@ def patch_variable(
raise HTTPException(
status.HTTP_400_BAD_REQUEST, "Invalid body, key from request body
doesn't match uri parameter"
)
- non_update_fields = {"key"}
variable =
session.scalar(select(Variable).filter_by(key=variable_key).limit(1))
if not variable:
raise HTTPException(
status.HTTP_404_NOT_FOUND, f"The Variable with key:
`{variable_key}` was not found"
)
- fields_to_update = patch_body.model_fields_set
- if update_mask:
- fields_to_update = fields_to_update.intersection(update_mask)
- else:
- try:
- VariableBody(**patch_body.model_dump())
- except ValidationError as e:
- raise RequestValidationError(errors=e.errors())
-
- data = patch_body.model_dump(include=fields_to_update - non_update_fields,
by_alias=True)
+ try:
+ VariableBody(**patch_body.model_dump())
Review Comment:
It would be nice to consolidate with `update_orm_from_pydantic` of Variable
like how we refactor the Pool's one by moving all the common logics to
`update_orm_from_pydantic`. Then the single patch variable endpoint can use
just `update_orm_from_pydantic` directly, and the bulk actions one can reuse it
as well.
##########
airflow-core/src/airflow/api_fastapi/core_api/routes/public/xcom.py:
##########
@@ -337,7 +332,6 @@ def update_xcom_entry(
f"The XCom with key: `{xcom_key}` with mentioned task instance
doesn't exist.",
)
- # Update XCom entry
Review Comment:
How about revert the change in
`airflow-core/src/airflow/api_fastapi/core_api/routes/public/xcom.py` as XCom
endpoint is not included in the scope.
--
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]