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: commits-unsubscr...@airflow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org