bbovenzi commented on code in PR #46504:
URL: https://github.com/apache/airflow/pull/46504#discussion_r1945247863
##########
airflow/ui/src/pages/Dag/Overview/Overview.tsx:
##########
@@ -42,14 +44,19 @@ export const Overview = () => {
state: ["failed"],
});
- const { data: failedRuns, isLoading: isLoadingRuns } =
useDagRunServiceGetDagRuns({
+ const { data: failedRuns, isLoading: isLoadingFailedRuns } =
useDagRunServiceGetDagRuns({
dagId: dagId ?? "",
logicalDateGte: startDate,
logicalDateLte: endDate,
state: ["failed"],
});
- // TODO actually link to task instances list
+ const { data: runs, isLoading: isLoadingRuns } = useDagRunServiceGetDagRuns({
+ dagId: dagId ?? "",
+ limit: 14,
Review Comment:
Yeah, it looks like there might be an issue in the rest api because I
definitely see the http requests sending the correct limit.
##########
airflow/ui/src/pages/Dag/Overview/RunDuration.tsx:
##########
@@ -0,0 +1,133 @@
+/*!
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+import { Box, Heading } from "@chakra-ui/react";
+import {
+ Chart as ChartJS,
+ CategoryScale,
+ LinearScale,
+ PointElement,
+ LineElement,
+ BarElement,
+ Filler,
+ Tooltip,
+} from "chart.js";
+import type { PartialEventContext } from "chartjs-plugin-annotation";
+import annotationPlugin from "chartjs-plugin-annotation";
+import dayjs from "dayjs";
+import { Bar } from "react-chartjs-2";
+
+import type { DAGRunResponse } from "openapi/requests/types.gen";
+import { getDuration } from "src/utils/datetime_utils";
+
+ChartJS.register(
+ CategoryScale,
+ LinearScale,
+ PointElement,
+ BarElement,
+ LineElement,
+ Filler,
+ Tooltip,
+ annotationPlugin,
+);
+
+const average = (ctx: PartialEventContext, index: number) => {
+ const values: Array<number> | undefined =
ctx.chart.data.datasets[index]?.data as Array<number> | undefined;
+
+ return values === undefined ? 0 : values.reduce((initial, next) => initial +
next, 0) / values.length;
+};
+
+export const RunDuration = ({ runs }: { readonly runs: Array<DAGRunResponse> |
undefined }) => {
+ if (!runs) {
+ return undefined;
+ }
+
+ const runAnnotation = {
+ borderColor: "green",
+ borderWidth: 1,
+ label: {
+ content: (ctx: PartialEventContext) => average(ctx, 1).toFixed(2),
+ display: true,
+ position: "end",
+ },
+ scaleID: "y",
+ value: (ctx: PartialEventContext) => average(ctx, 1),
+ };
+
+ const queuedAnnotation = {
+ borderColor: "grey",
+ borderWidth: 1,
+ label: {
+ content: (ctx: PartialEventContext) => average(ctx, 0).toFixed(2),
+ display: true,
+ position: "end",
+ },
+ scaleID: "y",
+ value: (ctx: PartialEventContext) => average(ctx, 0),
+ };
+
+ return (
+ <Box>
+ <Heading pb={2} size="sm" textAlign="center">
+ Last 14 runs
Review Comment:
Let's make this `total_entries` instead of hardcoding a number.
##########
airflow/ui/src/pages/Dag/Overview/RunDuration.tsx:
##########
@@ -0,0 +1,133 @@
+/*!
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+import { Box, Heading } from "@chakra-ui/react";
+import {
+ Chart as ChartJS,
+ CategoryScale,
+ LinearScale,
+ PointElement,
+ LineElement,
+ BarElement,
+ Filler,
+ Tooltip,
+} from "chart.js";
+import type { PartialEventContext } from "chartjs-plugin-annotation";
+import annotationPlugin from "chartjs-plugin-annotation";
+import dayjs from "dayjs";
+import { Bar } from "react-chartjs-2";
+
+import type { DAGRunResponse } from "openapi/requests/types.gen";
+import { getDuration } from "src/utils/datetime_utils";
+
+ChartJS.register(
+ CategoryScale,
+ LinearScale,
+ PointElement,
+ BarElement,
+ LineElement,
+ Filler,
+ Tooltip,
+ annotationPlugin,
+);
+
+const average = (ctx: PartialEventContext, index: number) => {
+ const values: Array<number> | undefined =
ctx.chart.data.datasets[index]?.data as Array<number> | undefined;
+
+ return values === undefined ? 0 : values.reduce((initial, next) => initial +
next, 0) / values.length;
+};
+
+export const RunDuration = ({ runs }: { readonly runs: Array<DAGRunResponse> |
undefined }) => {
+ if (!runs) {
+ return undefined;
+ }
+
+ const runAnnotation = {
+ borderColor: "green",
+ borderWidth: 1,
+ label: {
+ content: (ctx: PartialEventContext) => average(ctx, 1).toFixed(2),
+ display: true,
+ position: "end",
+ },
+ scaleID: "y",
+ value: (ctx: PartialEventContext) => average(ctx, 1),
+ };
+
+ const queuedAnnotation = {
+ borderColor: "grey",
+ borderWidth: 1,
+ label: {
+ content: (ctx: PartialEventContext) => average(ctx, 0).toFixed(2),
+ display: true,
+ position: "end",
+ },
+ scaleID: "y",
+ value: (ctx: PartialEventContext) => average(ctx, 0),
+ };
+
+ return (
+ <Box>
+ <Heading pb={2} size="sm" textAlign="center">
+ Last 14 runs
+ </Heading>
+ <Bar
+ data={{
+ datasets: [
+ {
+ backgroundColor: "grey",
+ data: runs.map((run: DAGRunResponse) =>
Number(getDuration(run.queued_at, run.start_date))),
+ label: "Queued duration",
+ },
+ {
+ backgroundColor: runs.map((run: DAGRunResponse) => (run.state
=== "failed" ? "red" : "green")),
Review Comment:
Oh yes, we changed it all to the theme file and elsewhere in the UI when we
needed it we could just do `var(--chakra-colors-${taskInstance.state}-solid)`
but that doesn't work here.
We can manually get it from the theme file via:
```(system.tokens.categoryMap.get("colors")?.get(`${run.state}.600`)?.value```
but I really don't like that. Maybe we should pull out all the `600` values
of each task state in theme.ts to an exported object that we can easily import
here.
##########
airflow/ui/src/pages/Dag/Overview/Overview.tsx:
##########
@@ -42,14 +44,19 @@ export const Overview = () => {
state: ["failed"],
});
- const { data: failedRuns, isLoading: isLoadingRuns } =
useDagRunServiceGetDagRuns({
+ const { data: failedRuns, isLoading: isLoadingFailedRuns } =
useDagRunServiceGetDagRuns({
dagId: dagId ?? "",
logicalDateGte: startDate,
logicalDateLte: endDate,
state: ["failed"],
});
- // TODO actually link to task instances list
+ const { data: runs, isLoading: isLoadingRuns } = useDagRunServiceGetDagRuns({
+ dagId: dagId ?? "",
+ limit: 14,
Review Comment:
cc: @pierrejeambrun
##########
airflow/ui/src/pages/Dag/Overview/Overview.tsx:
##########
@@ -93,6 +100,13 @@ export const Overview = () => {
startDate={startDate}
/>
</HStack>
+ <Box h="1/3" my={5} w="1/3">
Review Comment:
Let's avoid doing percentage here. Let's use a grid layout instead and give
some minimum sizes for the graph.
--
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]
