Copilot commented on code in PR #62195:
URL: https://github.com/apache/airflow/pull/62195#discussion_r3030078493
##########
airflow-core/src/airflow/ui/src/pages/Run/Header.tsx:
##########
@@ -139,6 +141,10 @@ export const Header = ({ dagRun }: { readonly dagRun:
DAGRunResponse }) => {
/>
),
},
+ {
+ label: translate("dag:deadlineStatus.label"),
+ value: <DeadlineStatus dagId={dagId} dagRunId={dagRunId} />,
Review Comment:
This stat row is always added, but `DeadlineStatus` may render nothing
(returns `undefined`) when there are no alerts/deadlines. `HeaderCard`/`Stat`
will still render the label with an empty value, which is confusing. Consider
conditionally adding this stats entry only when there’s data to show, or have
`DeadlineStatus` return an explicit “none”/placeholder value instead of nothing.
```suggestion
value: (
<Box
as="span"
sx={{
"&:empty::before": {
color: "fg.muted",
content: `"${translate("none")}"`,
},
}}
>
<DeadlineStatus dagId={dagId} dagRunId={dagRunId} />
</Box>
),
```
##########
airflow-core/src/airflow/ui/src/pages/Run/DeadlineStatus.tsx:
##########
@@ -0,0 +1,103 @@
+/*!
+ * 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 { Badge, HStack, Text, VStack } from "@chakra-ui/react";
+import { useTranslation } from "react-i18next";
+import { FiAlertTriangle, FiCheck, FiClock } from "react-icons/fi";
+
+import { useDeadlinesServiceGetDagDeadlineAlerts,
useDeadlinesServiceGetDeadlines } from "openapi/queries";
+import Time from "src/components/Time";
+
+type DeadlineStatusProps = {
+ readonly dagId: string;
+ readonly dagRunId: string;
+};
+
+export const DeadlineStatus = ({ dagId, dagRunId }: DeadlineStatusProps) => {
+ const { t: translate } = useTranslation("dag");
+
+ const { data: deadlineData } = useDeadlinesServiceGetDeadlines({
+ dagId,
+ dagRunId,
+ limit: 10,
+ orderBy: ["deadline_time"],
+ });
+
+ const { data: alertData } = useDeadlinesServiceGetDagDeadlineAlerts({ dagId
});
+
+ const deadlines = deadlineData?.deadlines ?? [];
+ const hasAlerts = (alertData?.total_entries ?? 0) > 0;
+
+ if (deadlines.length === 0 && !hasAlerts) {
+ return undefined;
+ }
+
+ if (deadlines.length === 0 && hasAlerts) {
+ return (
+ <HStack gap={1}>
+ <FiCheck color="var(--chakra-colors-fg-success)" />
+ <Badge colorPalette="green" size="sm" variant="solid">
+ {translate("deadlineStatus.met")}
+ </Badge>
+ </HStack>
+ );
Review Comment:
`DeadlineStatus` bases its output on `deadlineData?.deadlines ?? []` and
`alertData?.total_entries ?? 0` without handling loading/error states. This can
render an incorrect status (e.g., “Met”) while deadlines are still loading, and
it can also render nothing on transient errors. Consider using the query
`isLoading/isError` flags (or gating on `data` being present) so the displayed
status is derived only from settled data (or show a small loading/unknown
state).
##########
airflow-core/src/airflow/ui/src/pages/Dag/Calendar/CalendarDeadlines.tsx:
##########
@@ -0,0 +1,122 @@
+/*!
+ * 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 { Badge, Box, Heading, HStack, Link, Separator, Text, VStack } from
"@chakra-ui/react";
+import { useTranslation } from "react-i18next";
+import { FiAlertTriangle, FiClock } from "react-icons/fi";
+import { Link as RouterLink } from "react-router-dom";
+
+import { useDeadlinesServiceGetDeadlines } from "openapi/queries";
+import type { DeadlineResponse } from "openapi/requests/types.gen";
+import Time from "src/components/Time";
+
+const DeadlineRow = ({
+ deadline,
+ translate,
+}: {
+ readonly deadline: DeadlineResponse;
+ readonly translate: (key: string) => string;
+}) => (
+ <HStack justifyContent="space-between" px={2} py={1.5} width="100%">
+ <VStack alignItems="flex-start" gap={0}>
+ <HStack>
+ {deadline.missed ? (
+ <FiAlertTriangle color="var(--chakra-colors-fg-error)" size={12} />
+ ) : (
+ <FiClock color="var(--chakra-colors-fg-info)" size={12} />
+ )}
+ <Badge colorPalette={deadline.missed ? "red" : "blue"} fontSize="2xs"
size="sm" variant="solid">
+ {deadline.missed ? translate("calendar.deadlines.missed") :
translate("calendar.deadlines.pending")}
+ </Badge>
+ <Link asChild color="fg.info" fontSize="sm">
+ <RouterLink
to={`/dags/${deadline.dag_id}/runs/${deadline.dag_run_id}`}>
+ {deadline.dag_run_id}
+ </RouterLink>
+ </Link>
+ </HStack>
+ {deadline.alert_name !== undefined && deadline.alert_name !== null &&
deadline.alert_name !== "" ? (
+ <Text color="fg.muted" fontSize="xs" pl={5}>
+ {deadline.alert_name}
+ </Text>
+ ) : undefined}
+ </VStack>
+ <Time datetime={deadline.deadline_time} fontSize="sm" />
+ </HStack>
+);
+
+type CalendarDeadlinesProps = {
+ readonly dagId: string;
+ readonly endDate: string;
+ readonly startDate: string;
+};
+
+export const CalendarDeadlines = ({ dagId, endDate, startDate }:
CalendarDeadlinesProps) => {
+ const { t: translate } = useTranslation("dag");
+
+ const { data: pendingData } = useDeadlinesServiceGetDeadlines({
+ dagId,
+ dagRunId: "~",
+ deadlineTimeGte: startDate,
+ deadlineTimeLte: endDate,
+ limit: 20,
+ missed: false,
+ orderBy: ["deadline_time"],
+ });
+
+ const { data: missedData } = useDeadlinesServiceGetDeadlines({
+ dagId,
+ dagRunId: "~",
+ deadlineTimeGte: startDate,
+ deadlineTimeLte: endDate,
+ limit: 20,
+ missed: true,
+ orderBy: ["-deadline_time"],
+ });
+
+ const pendingDeadlines = pendingData?.deadlines ?? [];
+ const missedDeadlines = missedData?.deadlines ?? [];
+ const allDeadlines = [...missedDeadlines, ...pendingDeadlines];
+
+ if (allDeadlines.length === 0) {
+ return undefined;
+ }
+
+ return (
+ <Box borderRadius="lg" borderWidth={1} mt={6} p={4}>
+ <HStack mb={3}>
+ <FiClock />
+ <Heading size="xs">{translate("calendar.deadlines.title")}</Heading>
+ {missedDeadlines.length > 0 ? (
+ <Badge colorPalette="red" size="sm" variant="solid">
+ {missedDeadlines.length} {translate("calendar.deadlines.missed")}
+ </Badge>
+ ) : undefined}
+ {pendingDeadlines.length > 0 ? (
+ <Badge colorPalette="blue" size="sm" variant="solid">
+ {pendingDeadlines.length} {translate("calendar.deadlines.pending")}
+ </Badge>
+ ) : undefined}
Review Comment:
This component hard-limits each query to 20 items but uses
`missedDeadlines.length`/`pendingDeadlines.length` for the displayed counts.
When total entries exceed 20, the UI will undercount and the list will silently
truncate. Consider using `pendingData.total_entries`/`missedData.total_entries`
for counts and indicating truncation (or fetching all pages if that’s intended).
##########
airflow-core/src/airflow/ui/src/pages/Dag/DeadlineAlertsBadge.tsx:
##########
@@ -0,0 +1,101 @@
+/*!
+ * 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, Button, HStack, Separator, Text, VStack } from
"@chakra-ui/react";
+import { useTranslation } from "react-i18next";
+import { FiClock } from "react-icons/fi";
+
+import { useDeadlinesServiceGetDagDeadlineAlerts } from "openapi/queries";
+import type { DeadlineAlertResponse } from "openapi/requests/types.gen";
+import { Popover } from "src/components/ui";
+import { renderDuration } from "src/utils";
+
+const referenceTypeLabels: Record<string, string> = {
+ AverageRuntimeDeadline: "Average Runtime",
+ DagRunLogicalDateDeadline: "Logical Date",
+ DagRunQueuedAtDeadline: "Queued At",
+ FixedDatetimeDeadline: "Fixed Datetime",
+};
+
+const formatReferenceType = (referenceType: string): string =>
+ referenceTypeLabels[referenceType] ?? referenceType;
+
+const AlertRow = ({ alert }: { readonly alert: DeadlineAlertResponse }) => {
+ const { t: translate } = useTranslation("dag");
+
+ return (
+ <Box py={2} width="100%">
+ <Text fontWeight="bold">
+ {alert.name !== undefined && alert.name !== null && alert.name !== ""
+ ? alert.name
+ : translate("deadlineAlerts.unnamed")}
+ </Text>
+ {alert.description !== undefined && alert.description !== null &&
alert.description !== "" ? (
+ <Text color="fg.muted" fontSize="xs">
+ {alert.description}
+ </Text>
+ ) : undefined}
+ <HStack fontSize="xs" gap={3} mt={1}>
+ <Text color="fg.muted">
+ {translate("deadlineAlerts.referenceType")}:
{formatReferenceType(alert.reference_type)}
+ </Text>
+ <Text color="fg.muted">
+ {translate("deadlineAlerts.interval")}:{" "}
+ {renderDuration(alert.interval, false) ?? `${alert.interval}s`}
+ </Text>
+ </HStack>
+ </Box>
+ );
+};
+
+export const DeadlineAlertsBadge = ({ dagId }: { readonly dagId: string }) => {
+ const { t: translate } = useTranslation("dag");
+
+ const { data } = useDeadlinesServiceGetDagDeadlineAlerts({ dagId });
+
+ const alerts = data?.deadline_alerts ?? [];
+
+ if (alerts.length === 0) {
+ return undefined;
+ }
+
+ return (
+ // eslint-disable-next-line jsx-a11y/no-autofocus
+ <Popover.Root autoFocus={false} lazyMount unmountOnExit>
+ <Popover.Trigger asChild>
+ <Button size="xs" variant="outline">
+ <FiClock />
+ {translate("deadlineAlerts.count", { count: alerts.length })}
+ </Button>
+ </Popover.Trigger>
Review Comment:
The badge uses `alerts.length` for the displayed count and renders only the
(implicitly) paginated first page. If there are more alerts than the API page
limit, the count and list will be incomplete/misleading. Consider using
`data.total_entries` for the count and either increasing the requested limit or
adding pagination/“show more” behavior.
##########
airflow-core/src/airflow/models/serialized_dag.py:
##########
@@ -502,6 +502,8 @@ def _create_deadline_alert_records(
for uuid_str, deadline_data in uuid_mapping.items():
alert = DeadlineAlertModel(
id=UUID(uuid_str),
+ name=deadline_data.get(DeadlineAlertFields.NAME),
+ description=deadline_data.get(DeadlineAlertFields.DESCRIPTION),
reference=deadline_data[DeadlineAlertFields.REFERENCE],
interval=deadline_data[DeadlineAlertFields.INTERVAL],
callback_def=deadline_data[DeadlineAlertFields.CALLBACK],
Review Comment:
DeadlineAlert UUID reuse logic treats only (reference, interval, callback)
as the “definition”. With name/description now persisted to the DB, changing
just name/description will incorrectly be treated as “unchanged”, so existing
DeadlineAlert rows won’t be updated and the UI/API will keep showing stale
values. Consider including NAME/DESCRIPTION in the definition match/reuse
decision (or explicitly updating those columns when reusing UUIDs).
--
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]
