You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by "tirkarthi (via GitHub)" <gi...@apache.org> on 2025/02/06 08:10:18 UTC
[PR] Add recent runs duration chart. [airflow]
tirkarthi opened a new pull request, #46504:
URL: https://github.com/apache/airflow/pull/46504
This uses the API to get recent 14 dag runs to plot the chart along with average of the run and queued duration as annotations. The size is 1/3 since there will be 2 more for success/failed rate and grouped durations as per design. This is for initial implementation and might not completely cover all use cases like Airflow 2 but can be taken up in subsequent PRs or separate issues for them as below. If approved a similar overview page can be created for task page as well in another PR for task duration.
Notes to self and review
1. It seems `stateColor.ts` was removed. What is the recommended way to show color based on state for a given theme?
2. Legend is not implemented now where in Airflow 2 it can be selected to show only those values.
3. Duration is now in seconds and conversion to other units is yet to be implemented.
4. x axis value is set to 3 since high number causes values to render vertically amidst smaller space.
5. landing times based toggle is not implemented yet.
6. Clicking on dagrun to take to the run page is not implemented yet.
7. Autorefresh is not yet implemented.
8. It seems even with limit of 14 passed more dagruns are returned. Changing it manually reflects correct value. Need to see if there is some cache issue.
--
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-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add recent runs duration chart. [airflow]
Posted by "bbovenzi (via GitHub)" <gi...@apache.org>.
bbovenzi commented on code in PR #46504:
URL: https://github.com/apache/airflow/pull/46504#discussion_r1946994490
##########
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:
Actually, I was doing some user testing here. I think we should use the startDate and endDate variables from the TimeRange selectors in this query. As a user, I would expect changing my time range to change the graph beneath it.
--
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-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add recent runs duration chart. [airflow]
Posted by "tirkarthi (via GitHub)" <gi...@apache.org>.
tirkarthi commented on code in PR #46504:
URL: https://github.com/apache/airflow/pull/46504#discussion_r1947060884
##########
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:
Sure, created https://github.com/apache/airflow/issues/46572 .
--
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-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add recent runs duration chart. [airflow]
Posted by "bbovenzi (via GitHub)" <gi...@apache.org>.
bbovenzi commented on code in PR #46504:
URL: https://github.com/apache/airflow/pull/46504#discussion_r1946694082
##########
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:
<img width="520" alt="Screenshot 2025-02-07 at 10 14 43 AM" src="https://github.com/user-attachments/assets/f8d394dd-d1d2-48f9-ae20-240009c2f340" />
I was looking at the designs again. We definitely should make this fit in a grid layout
--
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-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add recent runs duration chart. [airflow]
Posted by "tirkarthi (via GitHub)" <gi...@apache.org>.
tirkarthi commented on code in PR #46504:
URL: https://github.com/apache/airflow/pull/46504#discussion_r1946936393
##########
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:
Thanks, done.
--
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-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add recent runs duration chart. [airflow]
Posted by "tirkarthi (via GitHub)" <gi...@apache.org>.
tirkarthi commented on code in PR #46504:
URL: https://github.com/apache/airflow/pull/46504#discussion_r1946939784
##########
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:
Thanks, made it a grid. I initially had it as a grid in other branch but used box since the other 2 charts needed API. Added border and padding.
--
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-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add recent runs duration chart. [airflow]
Posted by "tirkarthi (via GitHub)" <gi...@apache.org>.
tirkarthi commented on code in PR #46504:
URL: https://github.com/apache/airflow/pull/46504#discussion_r1947018763
##########
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:
I tried below and it's still returning 17 entries which are the total entries as per the network tab response.
```js
const { data: runs, isLoading: isLoadingRuns } = useDagRunServiceGetDagRuns({
dagId: dagId ?? "",
limit: 14,
offset: 0,
orderBy: "-start_date",
});
```
--
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-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add recent runs duration chart. [airflow]
Posted by "bbovenzi (via GitHub)" <gi...@apache.org>.
bbovenzi commented on code in PR #46504:
URL: https://github.com/apache/airflow/pull/46504#discussion_r1947031916
##########
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:
Alright, do you care to open an issue for this on the AIP-84 project board? Let's not have it block this 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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add recent runs duration chart. [airflow]
Posted by "bbovenzi (via GitHub)" <gi...@apache.org>.
bbovenzi commented on code in PR #46504:
URL: https://github.com/apache/airflow/pull/46504#discussion_r1946991476
##########
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:
Cool! We can move the trend buttons into the grid too whenever we have a bit more going on on this page.
--
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-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add recent runs duration chart. [airflow]
Posted by "tirkarthi (via GitHub)" <gi...@apache.org>.
tirkarthi commented on PR #46504:
URL: https://github.com/apache/airflow/pull/46504#issuecomment-2644001638
The link was to a sample task details page which now only has task instances and events tab. My understanding from the designs is that there will be a overview page for a task similar to dag page. The task duration chart was useful with median plot and it will be good to have it somewhere in Airflow 3 too.
--
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-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add recent runs duration chart. [airflow]
Posted by "pierrejeambrun (via GitHub)" <gi...@apache.org>.
pierrejeambrun merged PR #46504:
URL: https://github.com/apache/airflow/pull/46504
--
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-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add recent runs duration chart. [airflow]
Posted by "tirkarthi (via GitHub)" <gi...@apache.org>.
tirkarthi commented on code in PR #46504:
URL: https://github.com/apache/airflow/pull/46504#discussion_r1946944802
##########
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:
Thanks, just used `(system.tokens.categoryMap.get("colors")?.get(`${run.state}.600`)?.value` and also for `queued` state for simplicity since I feel the refactor might expand the scope of the 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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add recent runs duration chart. [airflow]
Posted by "bbovenzi (via GitHub)" <gi...@apache.org>.
bbovenzi commented on code in PR #46504:
URL: https://github.com/apache/airflow/pull/46504#discussion_r1947009592
##########
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:
Actually, add `offset: 0`. I think that's why the limit isn't being respected.
--
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-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add recent runs duration chart. [airflow]
Posted by "bbovenzi (via GitHub)" <gi...@apache.org>.
bbovenzi commented on code in PR #46504:
URL: https://github.com/apache/airflow/pull/46504#discussion_r1946972299
##########
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:
Works for me. If we run into this more, I'll handle the refactor.
--
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-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add recent runs duration chart. [airflow]
Posted by "bbovenzi (via GitHub)" <gi...@apache.org>.
bbovenzi commented on PR #46504:
URL: https://github.com/apache/airflow/pull/46504#issuecomment-2643910473
> Thanks @bbovenzi , I suppose a very similar overview page will be present for each task too with recent failed task instances, task duration chart etc, as per the design.
>
> http://localhost:8000/webapp/dags/tutorial_taskflow_api/tasks/load
Yes, eventually we want to do a lot more with these dashboard views on the homepage and on a Dag or Task.
What were you trying to link here?
--
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-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add recent runs duration chart. [airflow]
Posted by "bbovenzi (via GitHub)" <gi...@apache.org>.
bbovenzi commented on code in PR #46504:
URL: https://github.com/apache/airflow/pull/46504#discussion_r1946997765
##########
airflow/ui/src/pages/Dag/Overview/Overview.tsx:
##########
@@ -93,6 +100,15 @@ export const Overview = () => {
startDate={startDate}
/>
</HStack>
+ <SimpleGrid columns={3} gap={5} my={5}>
+ <Box borderRadius={4} borderStyle="solid" borderWidth={1} p={2}>
+ {isLoadingRuns ? (
+ <Skeleton height="200px" w="full" />
+ ) : (
+ <RunDuration runs={runs?.dag_runs} totalEntries={runs?.total_entries ?? 0} />
Review Comment:
If there are no runs, we should display a message like "No runs for this date range"
--
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-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add recent runs duration chart. [airflow]
Posted by "pierrejeambrun (via GitHub)" <gi...@apache.org>.
pierrejeambrun commented on code in PR #46504:
URL: https://github.com/apache/airflow/pull/46504#discussion_r1948979669
##########
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:
PR: https://github.com/apache/airflow/pull/46619
--
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-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add recent runs duration chart. [airflow]
Posted by "tirkarthi (via GitHub)" <gi...@apache.org>.
tirkarthi commented on code in PR #46504:
URL: https://github.com/apache/airflow/pull/46504#discussion_r1946946424
##########
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:
Yes, it seems the filters like sort are also not applied in the API end. I am not sure why it's specific to this component since other pages also have multiple API calls and components.
--
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-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add recent runs duration chart. [airflow]
Posted by "tirkarthi (via GitHub)" <gi...@apache.org>.
tirkarthi commented on PR #46504:
URL: https://github.com/apache/airflow/pull/46504#issuecomment-2643827586
Thanks @bbovenzi , I suppose a very similar overview page will be present for each task too with recent failed task instances, task duration chart etc, as per the design.
http://localhost:8000/webapp/dags/tutorial_taskflow_api/tasks/load
--
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-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add recent runs duration chart. [airflow]
Posted by "bbovenzi (via GitHub)" <gi...@apache.org>.
bbovenzi commented on code in PR #46504:
URL: https://github.com/apache/airflow/pull/46504#discussion_r1946994490
##########
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:
Actually, I was doing some user testing here. I think we should use the startDate and endDate variables from the TimeRange selectors in this query. As a user, I would expect changing my time range to change the graph beneath it.
--
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-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add recent runs duration chart. [airflow]
Posted by "bbovenzi (via GitHub)" <gi...@apache.org>.
bbovenzi commented on code in PR #46504:
URL: https://github.com/apache/airflow/pull/46504#discussion_r1946997765
##########
airflow/ui/src/pages/Dag/Overview/Overview.tsx:
##########
@@ -93,6 +100,15 @@ export const Overview = () => {
startDate={startDate}
/>
</HStack>
+ <SimpleGrid columns={3} gap={5} my={5}>
+ <Box borderRadius={4} borderStyle="solid" borderWidth={1} p={2}>
+ {isLoadingRuns ? (
+ <Skeleton height="200px" w="full" />
+ ) : (
+ <RunDuration runs={runs?.dag_runs} totalEntries={runs?.total_entries ?? 0} />
Review Comment:
If there are no runs, we should display a message like "No runs for this date range"
--
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-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add recent runs duration chart. [airflow]
Posted by "bbovenzi (via GitHub)" <gi...@apache.org>.
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org