Copilot commented on code in PR #64103:
URL: https://github.com/apache/airflow/pull/64103#discussion_r3066483019
##########
airflow-core/src/airflow/ui/src/components/DurationChart.tsx:
##########
@@ -121,165 +145,151 @@ export const DurationChart = ({
}
});
- const runAnnotation = {
- borderColor: "grey",
- borderWidth: 1,
- label: {
- content: (ctx: PartialEventContext) => renderDuration(average(ctx, 1),
false) ?? "0",
- display: true,
- position: "end",
- },
- scaleID: "y",
- value: (ctx: PartialEventContext) => average(ctx, 1),
- };
+ const runAnnotation = mkAnnotation({
+ color: "grey",
+ labelFn: (ctx) => renderDuration(average(ctx, 1), false) ?? "0",
+ valueFn: (ctx) => average(ctx, 1),
+ });
+ const queuedAnnotation = mkAnnotation({
+ color: "grey",
+ labelFn: (ctx) => renderDuration(average(ctx, 0), false) ?? "0",
+ valueFn: (ctx) => average(ctx, 0),
+ });
+ const medianAnnotation = mkAnnotation({
+ color: "blue",
+ dash: [6, 3],
+ labelFn: (ctx) => `Median: ${renderDuration(median(ctx, 1), false) ??
"0"}`,
+ valueFn: (ctx) => median(ctx, 1),
+ });
- const queuedAnnotation = {
- borderColor: "grey",
- borderWidth: 1,
- label: {
- content: (ctx: PartialEventContext) => renderDuration(average(ctx, 0),
false) ?? "0",
- display: true,
- position: "end",
- },
- scaleID: "y",
- value: (ctx: PartialEventContext) => average(ctx, 0),
+ const getQueuedDuration = (entry: RunResponse) => {
+ switch (kind) {
+ case "Dag Run": {
+ const run = entry as GridRunsResponse;
+
+ return run.queued_at !== null && run.start_date !== null &&
run.queued_at < run.start_date
+ ? Number(getDuration(run.queued_at, run.start_date))
+ : 0;
+ }
+ case "Task Instance": {
+ const ti = entry as TaskInstanceResponse;
+
+ return ti.queued_when !== null && ti.start_date !== null &&
ti.queued_when < ti.start_date
+ ? Number(getDuration(ti.queued_when, ti.start_date))
+ : 0;
+ }
+ default:
+ return 0;
+ }
};
return (
- <Box>
+ <Box w="100%">
<Heading pb={2} size="sm" textAlign="center">
{kind === "Dag Run"
? translate("durationChart.lastDagRun", { count: entries.length })
: translate("durationChart.lastTaskInstance", { count:
entries.length })}
</Heading>
- <Bar
- data={{
- datasets: [
- {
- backgroundColor: getComputedCSSVariableValue(queuedColorToken ??
"oklch(0.5 0 0)"),
- data: entries.map((entry: RunResponse) => {
- switch (kind) {
- case "Dag Run": {
- const run = entry as GridRunsResponse;
-
- return run.queued_at !== null && run.start_date !== null
&& run.queued_at < run.start_date
- ? Number(getDuration(run.queued_at, run.start_date))
- : 0;
- }
- case "Task Instance": {
- const taskInstance = entry as TaskInstanceResponse;
+ <Box h="400px">
+ <Bar
+ data={{
+ datasets: [
+ {
+ backgroundColor: getComputedCSSVariableValue(queuedColorToken
?? "oklch(0.5 0 0)"),
+ data: entries.map((entry: RunResponse) =>
getQueuedDuration(entry)),
+ label: translate("durationChart.queuedDuration"),
+ },
+ {
+ backgroundColor: entries.map(
+ (entry: RunResponse) =>
+ (entry.state ? stateColorMap[entry.state] : undefined) ??
"oklch(0.5 0 0)",
+ ),
+ data: entries.map((entry: RunResponse) =>
+ entry.start_date === null ? 0 :
Number(getDuration(entry.start_date, entry.end_date)),
+ ),
+ label: translate("durationChart.runDuration"),
+ },
+ ],
+ labels: entries.map((entry: RunResponse) =>
+ dayjs(entry.run_after).format(DEFAULT_DATETIME_FORMAT),
+ ),
+ }}
+ datasetIdKey="id"
+ options={{
+ animation: isAutoRefreshing ? false : undefined,
+ maintainAspectRatio: false,
+ onClick: (_event, elements) => {
+ const [element] = elements;
+ return;
+ }
+ switch (kind) {
+ const entry = entries[element.index] as GridRunsResponse |
undefined;
- return taskInstance.queued_when !== null &&
- taskInstance.start_date !== null &&
- taskInstance.queued_when < taskInstance.start_date
- ? Number(getDuration(taskInstance.queued_when,
taskInstance.start_date))
- : 0;
- }
- default:
- return 0;
+ void
Promise.resolve(navigate(`/dags/${entry?.dag_id}/runs/${entry?.run_id}`));
}
- }),
- label: translate("durationChart.queuedDuration"),
- },
- {
- backgroundColor: entries.map(
- (entry: RunResponse) =>
- (entry.state ? stateColorMap[entry.state] : undefined) ??
"oklch(0.5 0 0)",
- ),
- data: entries.map((entry: RunResponse) =>
- entry.start_date === null ? 0 :
Number(getDuration(entry.start_date, entry.end_date)),
- ),
- label: translate("durationChart.runDuration"),
- },
- ],
- labels: entries.map((entry: RunResponse) =>
dayjs(entry.run_after).format(DEFAULT_DATETIME_FORMAT)),
- }}
- datasetIdKey="id"
- options={{
- animation: isAutoRefreshing ? false : undefined,
- onClick: (_event, elements) => {
- const [element] = elements;
-
- if (!element) {
- return;
- }
+ case "Task Instance": {
+ const entry = entries[element.index] as TaskInstanceResponse
| undefined;
Review Comment:
The `onClick` handler is syntactically broken (there’s a stray `return;` and
unmatched braces), and the `switch (kind)` is missing a `case "Dag Run"`
label/`break`. This will fail TypeScript compilation and prevent the chart from
rendering/navigating. Restore the `if (!element) { return; }` guard and ensure
the switch cases/braces are correctly structured for both kinds.
##########
airflow-core/src/airflow/ui/src/components/DurationChart.tsx:
##########
@@ -18,13 +18,14 @@
*/
import { Box, Heading, useToken } from "@chakra-ui/react";
import {
+ BarElement,
Chart as ChartJS,
CategoryScale,
+ Filler,
+ Legend,
LinearScale,
- PointElement,
LineElement,
- BarElement,
- Filler,
+ PointElement,
Tooltip,
} from "chart.js";
Review Comment:
PR description/title discuss preventing credential leakage in GitHook, but
this diff only changes a UI chart component (`DurationChart.tsx`) and doesn’t
touch any GitHook/credential-handling code. Please align the PR
description/title with the actual changes, or move these UI changes to a
separate PR.
--
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]
