uranusjr commented on code in PR #51295:
URL: https://github.com/apache/airflow/pull/51295#discussion_r2122433778


##########
airflow-core/src/airflow/api_fastapi/execution_api/routes/task_instances.py:
##########
@@ -374,39 +377,78 @@ def ti_update_state(
 
     # We exclude_unset to avoid updating fields that are not set in the payload
     data = ti_patch_payload.model_dump(exclude={"task_outlets", 
"outlet_events"}, exclude_unset=True)
-
     query = update(TI).where(TI.id == ti_id_str).values(data)
 
-    if isinstance(ti_patch_payload, TITerminalStatePayload):
-        updated_state = ti_patch_payload.state
-        query = TI.duration_expression_update(ti_patch_payload.end_date, 
query, session.bind)
-        query = query.values(state=updated_state)
+    try:
+        query, updated_state = _create_ti_state_update_query_and_update_state(
+            ti_patch_payload=ti_patch_payload,
+            ti_id_str=ti_id_str,
+            dag_bag=dag_bag,
+            session=session,
+            query=query,
+            updated_state=updated_state,
+            dag_id=dag_id,
+        )
+    except Exception as err:
+        # This is an expected exception for MySQL
+        if isinstance(err, HTTPException) and err.status_code == 
status.HTTP_422_UNPROCESSABLE_ENTITY:
+            raise
 
-        if updated_state == TerminalTIState.FAILED:
-            ti = session.get(TI, ti_id_str)
-            ser_dag = dag_bag.get_dag(dag_id)
-            if ser_dag and getattr(ser_dag, "fail_fast", False):
-                task_dict = getattr(ser_dag, "task_dict")
-                task_teardown_map = {k: v.is_teardown for k, v in 
task_dict.items()}
-                _stop_remaining_tasks(task_instance=ti, 
task_teardown_map=task_teardown_map, session=session)
-
-    elif isinstance(ti_patch_payload, TIRetryStatePayload):
+        # Set a task to failed in case any unexpected exception happened 
during task state update
+        log.exception("Error updating Task Instance state to %s. Set the task 
to failed", updated_state)
+        ti = session.get(TI, ti_id_str)
+        query = TI.duration_expression_update(datetime.now(tz=timezone.utc), 
query, session.bind)
+        query = query.values(state=TaskInstanceState.FAILED)
+        _handle_fail_fast_for_dag(ti=ti, dag_id=dag_id, session=session, 
dag_bag=dag_bag)
+
+    # TODO: Replace this with FastAPI's Custom Exception handling:
+    # 
https://fastapi.tiangolo.com/tutorial/handling-errors/#install-custom-exception-handlers
+    try:
+        result = session.execute(query)
+        log.info("Task instance state updated", new_state=updated_state, 
rows_affected=result.rowcount)
+    except SQLAlchemyError as e:
+        log.error("Error updating Task Instance state", error=str(e))
+        raise HTTPException(
+            status_code=status.HTTP_500_INTERNAL_SERVER_ERROR, 
detail="Database error occurred"
+        )

Review Comment:
   What happens if we don’t raise an HTTPException but just reraise? (the 
behaviour before this change?)



-- 
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]

Reply via email to