pierrejeambrun commented on code in PR #56760:
URL: https://github.com/apache/airflow/pull/56760#discussion_r2577683278
##########
airflow-core/src/airflow/api_fastapi/core_api/datamodels/hitl.py:
##########
@@ -101,3 +102,9 @@ class HITLDetailCollection(BaseModel):
hitl_details: Iterable[HITLDetail]
total_entries: int
+
+
+class HITLDetailHisotry(BaseHITLDetail):
Review Comment:
Needs to be deleted, this doesn't exist anymore, it's `HITLDetailHistory`
##########
airflow-core/src/airflow/ui/src/pages/TaskInstance/HITLResponse.tsx:
##########
@@ -18,26 +18,65 @@
*/
import { Box } from "@chakra-ui/react";
import { useParams } from "react-router-dom";
+import { useSearchParams } from "react-router-dom";
-import { useTaskInstanceServiceGetHitlDetail } from "openapi/queries";
+import {
+ useTaskInstanceServiceGetHitlDetailTryDetail,
+ useTaskInstanceServiceGetMappedTaskInstance,
+} from "openapi/queries";
+import { TaskTrySelect } from "src/components/TaskTrySelect";
import { ProgressBar } from "src/components/ui";
+import { SearchParamsKeys } from "src/constants/searchParams";
+import { isStatePending, useAutoRefresh } from "src/utils";
import { HITLResponseForm } from "../HITLTaskInstances/HITLResponseForm";
export const HITLResponse = () => {
const { dagId, mapIndex, runId, taskId } = useParams();
- const { data: hitlDetail } = useTaskInstanceServiceGetHitlDetail(
+ const refetchInterval = useAutoRefresh({ dagId });
+ const [searchParams, setSearchParams] = useSearchParams();
+ const tryNumberParam = searchParams.get(SearchParamsKeys.TRY_NUMBER);
+
+ const parsedMapIndex = Number(mapIndex ?? -1);
+
+ const { data: taskInstance } = useTaskInstanceServiceGetMappedTaskInstance(
+ {
+ dagId: dagId ?? "",
+ dagRunId: runId ?? "",
+ mapIndex: parsedMapIndex,
+ taskId: taskId ?? "",
+ },
+ undefined,
+ {
+ enabled: !isNaN(parsedMapIndex),
+ refetchInterval: (query) => (isStatePending(query.state.data?.state) ?
refetchInterval : false),
+ },
+ );
+
+ const onSelectTryNumber = (newTryNumber: number) => {
+ if (newTryNumber === taskInstance?.try_number) {
+ searchParams.delete(SearchParamsKeys.TRY_NUMBER);
+ } else {
+ searchParams.set(SearchParamsKeys.TRY_NUMBER, newTryNumber.toString());
+ }
+ setSearchParams(searchParams);
+ };
+
+ const tryNumber = tryNumberParam === null ? -1 : parseInt(tryNumberParam,
10);
Review Comment:
This `-1` for try number seems odd. It should either be a valid try number
>=1 or `null/undefined`. -1 feels like a placeholder value
##########
airflow-core/src/airflow/api_fastapi/core_api/routes/public/hitl.py:
##########
@@ -72,21 +75,31 @@ def _get_task_instance_with_hitl_detail(
task_id: str,
session: SessionDep,
map_index: int,
-) -> TI:
- query = (
- select(TI)
- .where(
- TI.dag_id == dag_id,
- TI.run_id == dag_run_id,
- TI.task_id == task_id,
+ try_number: int | None = None,
+) -> TI | TIH:
+ def _query(orm_object: Base) -> TI | TIH | None:
+ query = (
+ select(orm_object)
+ .where(
+ orm_object.dag_id == dag_id,
+ orm_object.run_id == dag_run_id,
+ orm_object.task_id == task_id,
+ orm_object.map_index == map_index,
+ )
+ .options(joinedload(orm_object.hitl_detail))
)
- .options(joinedload(TI.hitl_detail))
- )
- if map_index is not None:
- query = query.where(TI.map_index == map_index)
+ if try_number is not None:
+ query = query.where(orm_object.try_number == try_number)
+
+ task_instance = session.scalar(query)
Review Comment:
We can name this `rows`, `entities`, `models` anything not confusing us
about having TI while they can be TIH too.
> both of them are technically task instance
I don't understand this statement. Those are not the same model and don't
expose the same interface. If I expect a TI and do `get_task_instance` on this
I'll have a problem when it's a TIH.
Ideally we could fix both places. That's not a big deal though. (so we can
do that latter too, i'm fine)
--
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]
