tirkarthi commented on code in PR #38374:
URL: https://github.com/apache/airflow/pull/38374#discussion_r1535306736
##########
airflow/www/static/js/dag/details/task/TaskDuration.tsx:
##########
@@ -179,7 +170,18 @@ const TaskDuration = () => {
// @ts-ignore
dataset: {
dimensions: ["runId", "queuedDurationUnit", "runDurationUnit"],
- source: durations,
+ source: durations.map((duration) => {
+ if (duration) {
+ const durationInSeconds = duration as TaskInstanceDuration;
Review Comment:
In the initial PR it used to be that way using .push and foreach but I
remember vaguely that eslint had something about foreach and change was made to
map using TaskInstanceDuration[] and then null was added that later changed to
{}. Some of the relevant diffs as below. I guess this was made for a reason. I
would keep the cast as it is a simpler change to typecast like assert and use
the object. Thanks.
Relevant commits :
https://github.com/apache/airflow/commit/8156faa5f710b0e21ab741073d2a889e019240e6
https://github.com/apache/airflow/commit/23435af92dcaa4f0668978dbf103706d7c02acb0
```diff
- dagRuns.forEach((dagRun) => {
- const instance = hasTaskInstances.instances.find(
- (_instance) =>
- _instance.taskId === taskId && _instance.runId === dagRun.runId
- );
-
- if (!instance) return null;
+ const durations: TaskInstanceDuration[] = task.instances.map((instance)
=> {
// @ts-ignore
const runDuration = moment.duration(
instance?.startDate && instance?.endDate
@@ -108,77 +73,77 @@ const TaskDuration = () => {
maxDuration = runDuration.asSeconds();
}
- durations.push(
- new TaskInstanceDuration(
- instance,
- runDuration,
- queuedDuration,
- dagRun.dataIntervalStart,
- dagRun.dataIntervalEnd,
- dagRun.executionDate
- )
- );
-
- return null;
- });
```
```
- const durations: TaskInstanceDuration[] = task.instances.map((instance)
=> {
+ const durations: (TaskInstanceDuration | null)[] = dagRuns.map((dagRun)
=> {
+ const { runId } = dagRun;
+ const instance = task.instances.find((ti) => ti && ti.runId === runId);
+ if (!instance) return null;
```
```
diff --git a/airflow/www/static/js/dag/details/task/TaskDuration.tsx
b/airflow/www/static/js/dag/details/task/TaskDuration.tsx
index 1f99f39e4d..cb6ad1f69d 100644
--- a/airflow/www/static/js/dag/details/task/TaskDuration.tsx
+++ b/airflow/www/static/js/dag/details/task/TaskDuration.tsx
@@ -56,10 +56,10 @@ const TaskDuration = () => {
if (!task) return null;
const orderingLabel = ordering[0] || ordering[1] || "startDate";
- const durations: (TaskInstanceDuration | null)[] = dagRuns.map((dagRun)
=> {
+ const durations: (TaskInstanceDuration | {})[] = dagRuns.map((dagRun) => {
const { runId } = dagRun;
const instance = task.instances.find((ti) => ti && ti.runId === runId);
- if (!instance) return null;
+ if (!instance) return {};
```
https://github.com/github/eslint-plugin-github/blob/main/docs/rules/array-foreach.md
--
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
