bbovenzi commented on code in PR #70711:
URL: https://github.com/apache/airflow/pull/70711#discussion_r3707610467
##########
airflow-core/src/airflow/ui/src/components/TaskTrySelect.tsx:
##########
@@ -70,11 +70,23 @@ export const TaskTrySelect = ({ onSelectTryNumber,
selectedTryNumber, taskInstan
const logAttemptDropdownLimit = 10;
const showDropdown = finalTryNumber > logAttemptDropdownLimit;
- // For some reason tries aren't sorted by try_number
- const sortedTries = [...(tiHistory?.task_instances ?? [])].sort(
- (tryA, tryB) => tryA.try_number - tryB.try_number,
+ const triesByNumber = new Map(
+ (tiHistory?.task_instances ?? [])
+ .filter(
+ (ti) =>
+ ti.dag_id === dagId &&
+ ti.dag_run_id === dagRunId &&
+ ti.task_id === taskId &&
+ ti.map_index === mapIndex &&
+ ti.try_number > 0,
+ )
+ .map((ti) => [ti.try_number, ti]),
);
+ // The current task instance is authoritative when it is also present in
history.
+ triesByNumber.set(finalTryNumber, taskInstance);
Review Comment:
Let's also wrap this in finalTryNumber > 0 check
##########
airflow-core/src/airflow/ui/src/components/TaskTrySelect.tsx:
##########
@@ -70,11 +70,23 @@ export const TaskTrySelect = ({ onSelectTryNumber,
selectedTryNumber, taskInstan
const logAttemptDropdownLimit = 10;
const showDropdown = finalTryNumber > logAttemptDropdownLimit;
- // For some reason tries aren't sorted by try_number
- const sortedTries = [...(tiHistory?.task_instances ?? [])].sort(
- (tryA, tryB) => tryA.try_number - tryB.try_number,
+ const triesByNumber = new Map(
+ (tiHistory?.task_instances ?? [])
+ .filter(
+ (ti) =>
+ ti.dag_id === dagId &&
+ ti.dag_run_id === dagRunId &&
+ ti.task_id === taskId &&
+ ti.map_index === mapIndex &&
Review Comment:
These 4 filters should already be true
##########
airflow-core/src/airflow/ui/src/components/TaskTrySelect.tsx:
##########
@@ -70,11 +70,23 @@ export const TaskTrySelect = ({ onSelectTryNumber,
selectedTryNumber, taskInstan
const logAttemptDropdownLimit = 10;
const showDropdown = finalTryNumber > logAttemptDropdownLimit;
- // For some reason tries aren't sorted by try_number
- const sortedTries = [...(tiHistory?.task_instances ?? [])].sort(
- (tryA, tryB) => tryA.try_number - tryB.try_number,
+ const triesByNumber = new Map(
+ (tiHistory?.task_instances ?? [])
+ .filter(
+ (ti) =>
+ ti.dag_id === dagId &&
+ ti.dag_run_id === dagRunId &&
+ ti.task_id === taskId &&
+ ti.map_index === mapIndex &&
+ ti.try_number > 0,
+ )
+ .map((ti) => [ti.try_number, ti]),
);
+ // The current task instance is authoritative when it is also present in
history.
+ triesByNumber.set(finalTryNumber, taskInstance);
Review Comment:
Oh and we should also add a check to not set the final number when a
TI.state is `up_for_retry` or `null` since they are just placeholders
--
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]
