jscheffl commented on code in PR #53824:
URL: https://github.com/apache/airflow/pull/53824#discussion_r2237597264
##########
airflow-core/src/airflow/api_fastapi/execution_api/routes/hitl.py:
##########
@@ -40,32 +40,51 @@
"/{task_instance_id}",
status_code=status.HTTP_201_CREATED,
)
-def add_hitl_detail(
+def upsert_hitl_detail(
task_instance_id: UUID,
payload: HITLDetailRequest,
session: SessionDep,
) -> HITLDetailRequest:
- """Get Human-in-the-loop detail for a specific Task Instance."""
+ """
+ Create a Human-in-the-loop detail for a specific Task Instance.
+
+ There're 3 cases handled here.
+
+ 1. If a HITLOperator task instance does not have a HITLDetail,
+ a new HITLDetail is created without a response section.
+ 2. If a HITLOperator task instance has a HITLDetail but lacks a response,
+ the existing HITLDetail is returned.
+ This situation occurs when a task instance is cleared before a response
is received.
+ 3. If a HITLOperator task instance has both a HITLDetail and a response
section,
+ the existing response is removed, and the HITLDetail is returned.
+ This happens when a task instance is cleared after a response has been
received.
+ This design ensures that each task instance has only one HITLDetail.
+ """
ti_id_str = str(task_instance_id)
hitl_detail_model =
session.scalar(select(HITLDetail).where(HITLDetail.ti_id == ti_id_str))
- if hitl_detail_model:
- raise HTTPException(
- status.HTTP_409_CONFLICT,
- f"Human-in-the-loop detail for Task Instance with id {ti_id_str}
already exists.",
+ if not hitl_detail_model:
+ hitl_detail_model = HITLDetail(
+ ti_id=ti_id_str,
+ options=payload.options,
+ subject=payload.subject,
+ body=payload.body,
+ defaults=payload.defaults,
+ multiple=payload.multiple,
+ params=payload.params,
)
+ session.add(hitl_detail_model)
+ session.commit()
Review Comment:
In all the positive cases if the function returns normally you do not need
to commit()
```suggestion
```
##########
airflow-core/src/airflow/api_fastapi/execution_api/routes/hitl.py:
##########
@@ -40,32 +40,51 @@
"/{task_instance_id}",
status_code=status.HTTP_201_CREATED,
)
-def add_hitl_detail(
+def upsert_hitl_detail(
task_instance_id: UUID,
payload: HITLDetailRequest,
session: SessionDep,
) -> HITLDetailRequest:
- """Get Human-in-the-loop detail for a specific Task Instance."""
+ """
+ Create a Human-in-the-loop detail for a specific Task Instance.
+
+ There're 3 cases handled here.
+
+ 1. If a HITLOperator task instance does not have a HITLDetail,
+ a new HITLDetail is created without a response section.
+ 2. If a HITLOperator task instance has a HITLDetail but lacks a response,
+ the existing HITLDetail is returned.
+ This situation occurs when a task instance is cleared before a response
is received.
+ 3. If a HITLOperator task instance has both a HITLDetail and a response
section,
+ the existing response is removed, and the HITLDetail is returned.
+ This happens when a task instance is cleared after a response has been
received.
+ This design ensures that each task instance has only one HITLDetail.
+ """
ti_id_str = str(task_instance_id)
hitl_detail_model =
session.scalar(select(HITLDetail).where(HITLDetail.ti_id == ti_id_str))
- if hitl_detail_model:
- raise HTTPException(
- status.HTTP_409_CONFLICT,
- f"Human-in-the-loop detail for Task Instance with id {ti_id_str}
already exists.",
+ if not hitl_detail_model:
+ hitl_detail_model = HITLDetail(
+ ti_id=ti_id_str,
+ options=payload.options,
+ subject=payload.subject,
+ body=payload.body,
+ defaults=payload.defaults,
+ multiple=payload.multiple,
+ params=payload.params,
)
+ session.add(hitl_detail_model)
+ session.commit()
+ elif hitl_detail_model.response_received:
+ # Cleanup the response part of HITLDetail as we only store one
response for one task instance.
+ # It normally happens after retry, we keep only the latest response.
+ hitl_detail_model.user_id = None
+ hitl_detail_model.response_at = None
+ hitl_detail_model.chosen_options = None
+ hitl_detail_model.params_input = {}
+ session.add(hitl_detail_model)
+ session.commit()
Review Comment:
```suggestion
```
--
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]
