You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by "ephraimbuddy (via GitHub)" <gi...@apache.org> on 2026/09/18 11:07:24 UTC
[PR] Add an API endpoint and UI tab comparing two stored Dag versions [airflow]
ephraimbuddy opened a new pull request, #73322:
URL: https://github.com/apache/airflow/pull/73322
Adds the API and UI layer on top of the serialized Dag version diff engine: an
endpoint that compares two stored versions of a Dag, and a **Versions** tab that
renders the comparison.
The endpoint reports *observed state* — what the two stored versions currently
hold — rather than why a version was created. Disclosure is split in two:
`DagAccessEntity.VERSION` read returns which parts of the Dag differ with
identifying path components masked, while the values behind those differences,
and the paths naming them, are returned only to a caller who may also read the
Dag's code. A record carries a value key only when that side has one, which is
how an absent side stays distinguishable from a stored null.
The tab keeps both chosen versions and the change bound in the URL, so a
comparison can be linked and reopened.
### Stacked on #69864
This branch is based on `dag-version-diff-core`, so the diff here also contains
that PR's 20 commits. **Only the last 7 commits belong to this PR:**
- `0667dec76b` Add an API endpoint comparing two stored Dag versions
- `966f7a9386` Generate the API spec and clients for the Dag version diff
- `192e62901f` UI: Add a Versions tab comparing two Dag versions
- `7f54315417` UI: Move the version compare select to shared components
- `76d03764a2` UI: Make a Dag version comparison shareable and its limit adjustable
- `9c6bbb7440` Keep a Dag version diff's absent value distinct from a stored null
- `abe488a90b` UI: Fix a Dag version comparison misreporting its own results
---
##### Was generative AI tooling used to co-author this PR?
- [X] Yes — Claude Code (Opus 5)
Generated-by: Claude Code (Opus 5) following [the guidelines](https://github.com/apache/airflow/blob/main/contributing-docs/05_pull_requests.rst#gen-ai-assisted-contributions)
--
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 an API endpoint and UI tab comparing two stored Dag versions [airflow]
Posted by "ephraimbuddy (via GitHub)" <gi...@apache.org>.
ephraimbuddy commented on PR #73322:
URL: https://github.com/apache/airflow/pull/73322#issuecomment-5729165060
https://github.com/user-attachments/assets/b77982ff-ad5b-4dd1-801f-0815fd1b720f
--
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 an API endpoint and UI tab comparing two stored Dag versions [airflow]
Posted by "ephraimbuddy (via GitHub)" <gi...@apache.org>.
ephraimbuddy commented on code in PR #73322:
URL: https://github.com/apache/airflow/pull/73322#discussion_r4108452179
##########
airflow-ctl/src/airflowctl/api/datamodels/generated.py:
##########
@@ -2155,6 +2222,93 @@ class DagStatsResponse(BaseModel):
stats: Annotated[list[DagStatsStateResponse], Field(title="Stats")]
+class DagVersionDiffChangeResponse(BaseModel):
+ """
+ One structural difference between two stored Dag versions.
+ """
+
+ path: Annotated[str, Field(title="Path")]
+ operation: DagVersionDiffOperation
+ category: DagVersionDiffCategory
+ impact: DagVersionDiffImpact
+ occurrence_count: Annotated[
+ int,
+ Field(
+ description="How many underlying changes this record stands for. Always 1 when values are disclosed, since each change is then its own record; a redacted record merges every change sharing its path and operation.",
+ title="Occurrence Count",
+ ),
+ ]
+ before_digest: Annotated[
+ str | None,
+ Field(
+ description="SHA-256 over the canonical JSON of `before_value`. Present only when values are disclosed, and null when the change has no before side.",
+ title="Before Digest",
+ ),
+ ] = None
+ after_digest: Annotated[
+ str | None,
+ Field(
+ description="SHA-256 over the canonical JSON of `after_value`. Present only when values are disclosed, and null when the change has no after side.",
+ title="After Digest",
+ ),
+ ] = None
+ before_value: Annotated[
+ Any | None,
+ Field(
+ description="The value this path held in the base version. Present only when values are disclosed, and omitted entirely when the change has no before side — which is how an absent side is told apart from a stored null.",
+ title="Before Value",
+ ),
+ ] = None
+ after_value: Annotated[
+ Any | None,
+ Field(
+ description="The value this path holds in the target version. Present only when values are disclosed, and omitted entirely when the change has no after side — which is how an absent side is told apart from a stored null.",
+ title="After Value",
+ ),
+ ] = None
+
+
+class DagVersionDiffResponse(BaseModel):
+ """
+ Observed-state difference between two stored Dag versions.
+ """
+
+ diff_schema_version: Annotated[
+ int,
+ Field(
+ description="Wire format of this payload. Incremented when its shape changes.",
+ title="Diff Schema Version",
+ ),
+ ]
+ base_version_number: Annotated[int, Field(title="Base Version Number")]
+ target_version_number: Annotated[int, Field(title="Target Version Number")]
+ serialized_dag_schema_versions: DagVersionDiffSchemaVersions
Review Comment:
Fixed
--
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 an API endpoint and UI tab comparing two stored Dag versions [airflow]
Posted by "ephraimbuddy (via GitHub)" <gi...@apache.org>.
ephraimbuddy commented on code in PR #73322:
URL: https://github.com/apache/airflow/pull/73322#discussion_r4105255620
##########
airflow-core/src/airflow/api_fastapi/core_api/routes/public/dag_versions.py:
##########
@@ -132,3 +138,76 @@ def get_dag_versions(
dag_versions=dag_versions,
total_entries=total_entries,
)
+
+
+@dag_versions_router.get(
+ "/{base_version_number}/diff/{target_version_number}",
+ responses=create_openapi_http_exception_doc(
+ [
+ status.HTTP_400_BAD_REQUEST,
+ status.HTTP_404_NOT_FOUND,
+ ]
+ ),
+ dependencies=[Depends(requires_access_dag(method="GET", access_entity=DagAccessEntity.VERSION))],
+ # Serializing unset fields as null would erase the absent-vs-null distinction the value
+ # fields document.
+ response_model_exclude_unset=True,
+)
+def get_dag_version_diff(
Review Comment:
Also, we would have CLI version of this, so it would be better to keep it public for use by CI etc
--
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 an API endpoint and UI tab comparing two stored Dag versions [airflow]
Posted by "ephraimbuddy (via GitHub)" <gi...@apache.org>.
ephraimbuddy commented on code in PR #73322:
URL: https://github.com/apache/airflow/pull/73322#discussion_r4105156479
##########
airflow-core/src/airflow/api_fastapi/core_api/routes/public/dag_versions.py:
##########
@@ -132,3 +138,76 @@ def get_dag_versions(
dag_versions=dag_versions,
total_entries=total_entries,
)
+
+
+@dag_versions_router.get(
+ "/{base_version_number}/diff/{target_version_number}",
+ responses=create_openapi_http_exception_doc(
+ [
+ status.HTTP_400_BAD_REQUEST,
+ status.HTTP_404_NOT_FOUND,
+ ]
+ ),
+ dependencies=[Depends(requires_access_dag(method="GET", access_entity=DagAccessEntity.VERSION))],
+ # Serializing unset fields as null would erase the absent-vs-null distinction the value
+ # fields document.
+ response_model_exclude_unset=True,
+)
+def get_dag_version_diff(
Review Comment:
I think public is ok so that users can share the url for debugging.
--
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 an API endpoint and UI tab comparing two stored Dag versions [airflow]
Posted by "bbovenzi (via GitHub)" <gi...@apache.org>.
bbovenzi commented on PR #73322:
URL: https://github.com/apache/airflow/pull/73322#issuecomment-5938415774
Just need to rebase and regenerate docs
--
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 an API endpoint and UI tab comparing two stored Dag versions [airflow]
Posted by "pierrejeambrun (via GitHub)" <gi...@apache.org>.
pierrejeambrun commented on code in PR #73322:
URL: https://github.com/apache/airflow/pull/73322#discussion_r4104927210
##########
airflow-core/src/airflow/ui/src/components/VersionCompareSelect.tsx:
##########
@@ -49,9 +51,13 @@ export const VersionCompareSelect = ({
const selectedVersion = data?.dag_versions.find((dv) => dv.version_number === selectedVersionNumber);
- const versionOptions = createListCollection({
- items: (data?.dag_versions ?? []).map((dv) => ({ value: dv.version_number, version: dv })),
- });
+ const versionOptions = useMemo(
Review Comment:
I don't think we need that useMemo. React compiler will do that for us. `DagVersionSelect` call it directly too.
##########
airflow-ctl/src/airflowctl/api/datamodels/generated.py:
##########
@@ -2155,6 +2222,93 @@ class DagStatsResponse(BaseModel):
stats: Annotated[list[DagStatsStateResponse], Field(title="Stats")]
+class DagVersionDiffChangeResponse(BaseModel):
+ """
+ One structural difference between two stored Dag versions.
+ """
+
+ path: Annotated[str, Field(title="Path")]
+ operation: DagVersionDiffOperation
+ category: DagVersionDiffCategory
+ impact: DagVersionDiffImpact
+ occurrence_count: Annotated[
+ int,
+ Field(
+ description="How many underlying changes this record stands for. Always 1 when values are disclosed, since each change is then its own record; a redacted record merges every change sharing its path and operation.",
+ title="Occurrence Count",
+ ),
+ ]
+ before_digest: Annotated[
+ str | None,
+ Field(
+ description="SHA-256 over the canonical JSON of `before_value`. Present only when values are disclosed, and null when the change has no before side.",
+ title="Before Digest",
+ ),
+ ] = None
+ after_digest: Annotated[
+ str | None,
+ Field(
+ description="SHA-256 over the canonical JSON of `after_value`. Present only when values are disclosed, and null when the change has no after side.",
+ title="After Digest",
+ ),
+ ] = None
+ before_value: Annotated[
+ Any | None,
+ Field(
+ description="The value this path held in the base version. Present only when values are disclosed, and omitted entirely when the change has no before side — which is how an absent side is told apart from a stored null.",
+ title="Before Value",
+ ),
+ ] = None
+ after_value: Annotated[
+ Any | None,
+ Field(
+ description="The value this path holds in the target version. Present only when values are disclosed, and omitted entirely when the change has no after side — which is how an absent side is told apart from a stored null.",
+ title="After Value",
+ ),
+ ] = None
+
+
+class DagVersionDiffResponse(BaseModel):
+ """
+ Observed-state difference between two stored Dag versions.
+ """
+
+ diff_schema_version: Annotated[
+ int,
+ Field(
+ description="Wire format of this payload. Incremented when its shape changes.",
+ title="Diff Schema Version",
+ ),
+ ]
+ base_version_number: Annotated[int, Field(title="Base Version Number")]
+ target_version_number: Annotated[int, Field(title="Target Version Number")]
+ serialized_dag_schema_versions: DagVersionDiffSchemaVersions
Review Comment:
`DagVersionSerializerVersions` ? Cause it's confusing with `diff_schema_version` which is the diff algorithm version
##########
airflow-core/src/airflow/ui/src/pages/Dag/Versions/VersionDiff.tsx:
##########
@@ -0,0 +1,168 @@
+/*!
+ * 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 { Alert, Badge, Box, Code, Flex, Heading, Table, Text } from "@chakra-ui/react";
+import { useTranslation } from "react-i18next";
+
+import type { DagVersionDiffResponse } from "openapi/requests/types.gen";
+
+type VersionDiffProps = {
+ readonly baseVersionNumber: number;
+ readonly diff: DagVersionDiffResponse;
+ readonly targetVersionNumber: number;
+};
+
+const IMPACT_COLORS: Record<string, string> = {
+ authorization: "orange",
+ execution: "red",
+ metadata: "blue",
+ provenance: "gray",
+ unknown: "gray",
+};
+
+// A whole task added or removed carries its entire serialized payload, which would otherwise
+// unfold into one unreadable table cell.
+const MAX_VALUE_LENGTH = 120;
+
+const renderValue = (value: unknown) => {
+ if (value === undefined) {
+ return "—";
+ }
+
+ // A stored null is a value; only an absent side gets the dash above. An empty string is quoted
+ // so it reads as a value rather than as a blank cell.
+ const text = typeof value === "string" ? value || '""' : JSON.stringify(value);
+
+ return text.length > MAX_VALUE_LENGTH ? `${text.slice(0, MAX_VALUE_LENGTH)}…` : text;
+};
+
+export const VersionDiff = ({ baseVersionNumber, diff, targetVersionNumber }: VersionDiffProps) => {
+ const { t: translate } = useTranslation("dag");
+ const valuesShown = diff.values_status === "available";
+
+ if (diff.mode === "unavailable") {
+ return (
+ <Alert.Root status="info" title={translate("versions.unavailable.title")}>
+ <Alert.Description>
+ {diff.unavailable_reason === null || diff.unavailable_reason === undefined ? (
+ translate("versions.unavailable.withoutReason")
+ ) : (
+ <>
+ {translate("versions.unavailable.description")}{" "}
+ {/* A machine token, so it is shown as code rather than folded into a translated sentence. */}
+ <Code fontSize="sm">{diff.unavailable_reason}</Code>
+ </>
+ )}
+ </Alert.Description>
+ </Alert.Root>
+ );
+ }
+
+ return (
+ <Box>
+ <Flex alignItems="center" gap={3} justifyContent="space-between" mb={2}>
+ <Box>
+ <Heading size="md">
+ {translate("versions.heading", { base: baseVersionNumber, target: targetVersionNumber })}
+ </Heading>
+ <Text color="fg.muted" fontSize="sm">
+ {translate("versions.summary", {
+ baseSchema: diff.serialized_dag_schema_versions.base ?? "?",
+ count: diff.total_changes,
+ targetSchema: diff.serialized_dag_schema_versions.target ?? "?",
+ })}
+ </Text>
+ </Box>
+ <Flex alignItems="center" gap={2}>
+ <Badge colorPalette="teal">{translate("versions.observedState")}</Badge>
+ <Badge colorPalette={valuesShown ? "green" : "gray"}>
+ {translate(valuesShown ? "versions.valuesShown" : "versions.valuesHidden")}
+ </Badge>
+ {/* Says what grants values, since the viewer cannot grant it here: the server decides
+ from the caller's access to Dag code. */}
+ <Text color="fg.muted" fontSize="sm">
+ {translate("versions.codeAccess")}
+ </Text>
+ </Flex>
+ </Flex>
+
+ {diff.truncated ? (
+ <Alert.Root mb={2} status="warning" title={translate("versions.truncated.title")}>
+ <Alert.Description>{translate("versions.truncated.description")}</Alert.Description>
+ </Alert.Root>
+ ) : undefined}
+
+ {diff.changes.length === 0 ? (
+ <Text color="fg.muted">{translate("versions.noChanges")}</Text>
+ ) : (
+ <Table.Root striped>
Review Comment:
Probably better to use the shared `DataTable` component at our disposal instead of doing all that manually.
(for consistent styling and functionalities)
##########
airflow-core/src/airflow/api_fastapi/core_api/routes/public/dag_versions.py:
##########
@@ -132,3 +138,76 @@ def get_dag_versions(
dag_versions=dag_versions,
total_entries=total_entries,
)
+
+
+@dag_versions_router.get(
+ "/{base_version_number}/diff/{target_version_number}",
+ responses=create_openapi_http_exception_doc(
+ [
+ status.HTTP_400_BAD_REQUEST,
+ status.HTTP_404_NOT_FOUND,
+ ]
+ ),
+ dependencies=[Depends(requires_access_dag(method="GET", access_entity=DagAccessEntity.VERSION))],
+ # Serializing unset fields as null would erase the absent-vs-null distinction the value
+ # fields document.
+ response_model_exclude_unset=True,
+)
+def get_dag_version_diff(
Review Comment:
Should this be public API or UI only at this point ?
##########
airflow-core/src/airflow/api_fastapi/core_api/routes/public/dag_versions.py:
##########
@@ -132,3 +138,76 @@ def get_dag_versions(
dag_versions=dag_versions,
total_entries=total_entries,
)
+
+
+@dag_versions_router.get(
+ "/{base_version_number}/diff/{target_version_number}",
Review Comment:
No strong opinion but this tends to read as 'nested' resources in a REST convention.
Can we consider `dags/{dag_id}/dagVersions/diff?base={}&target={}` ?
Depends if we consider a 'diff' a subresource of a DagVersion, or `diffing` as an action between two versions.
WDYT?
##########
airflow-core/src/airflow/ui/src/pages/Dag/Versions/VersionDiff.tsx:
##########
@@ -0,0 +1,168 @@
+/*!
+ * 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 { Alert, Badge, Box, Code, Flex, Heading, Table, Text } from "@chakra-ui/react";
+import { useTranslation } from "react-i18next";
+
+import type { DagVersionDiffResponse } from "openapi/requests/types.gen";
+
+type VersionDiffProps = {
+ readonly baseVersionNumber: number;
+ readonly diff: DagVersionDiffResponse;
+ readonly targetVersionNumber: number;
+};
+
+const IMPACT_COLORS: Record<string, string> = {
+ authorization: "orange",
+ execution: "red",
+ metadata: "blue",
+ provenance: "gray",
+ unknown: "gray",
+};
+
+// A whole task added or removed carries its entire serialized payload, which would otherwise
+// unfold into one unreadable table cell.
+const MAX_VALUE_LENGTH = 120;
+
+const renderValue = (value: unknown) => {
+ if (value === undefined) {
+ return "—";
+ }
+
+ // A stored null is a value; only an absent side gets the dash above. An empty string is quoted
+ // so it reads as a value rather than as a blank cell.
+ const text = typeof value === "string" ? value || '""' : JSON.stringify(value);
+
+ return text.length > MAX_VALUE_LENGTH ? `${text.slice(0, MAX_VALUE_LENGTH)}…` : text;
+};
+
+export const VersionDiff = ({ baseVersionNumber, diff, targetVersionNumber }: VersionDiffProps) => {
+ const { t: translate } = useTranslation("dag");
+ const valuesShown = diff.values_status === "available";
+
+ if (diff.mode === "unavailable") {
+ return (
+ <Alert.Root status="info" title={translate("versions.unavailable.title")}>
+ <Alert.Description>
+ {diff.unavailable_reason === null || diff.unavailable_reason === undefined ? (
+ translate("versions.unavailable.withoutReason")
+ ) : (
+ <>
+ {translate("versions.unavailable.description")}{" "}
+ {/* A machine token, so it is shown as code rather than folded into a translated sentence. */}
+ <Code fontSize="sm">{diff.unavailable_reason}</Code>
+ </>
+ )}
+ </Alert.Description>
+ </Alert.Root>
+ );
+ }
+
+ return (
+ <Box>
+ <Flex alignItems="center" gap={3} justifyContent="space-between" mb={2}>
+ <Box>
+ <Heading size="md">
+ {translate("versions.heading", { base: baseVersionNumber, target: targetVersionNumber })}
+ </Heading>
+ <Text color="fg.muted" fontSize="sm">
+ {translate("versions.summary", {
+ baseSchema: diff.serialized_dag_schema_versions.base ?? "?",
+ count: diff.total_changes,
+ targetSchema: diff.serialized_dag_schema_versions.target ?? "?",
+ })}
+ </Text>
+ </Box>
+ <Flex alignItems="center" gap={2}>
+ <Badge colorPalette="teal">{translate("versions.observedState")}</Badge>
+ <Badge colorPalette={valuesShown ? "green" : "gray"}>
+ {translate(valuesShown ? "versions.valuesShown" : "versions.valuesHidden")}
+ </Badge>
+ {/* Says what grants values, since the viewer cannot grant it here: the server decides
+ from the caller's access to Dag code. */}
+ <Text color="fg.muted" fontSize="sm">
+ {translate("versions.codeAccess")}
+ </Text>
+ </Flex>
+ </Flex>
+
+ {diff.truncated ? (
+ <Alert.Root mb={2} status="warning" title={translate("versions.truncated.title")}>
+ <Alert.Description>{translate("versions.truncated.description")}</Alert.Description>
+ </Alert.Root>
+ ) : undefined}
+
+ {diff.changes.length === 0 ? (
+ <Text color="fg.muted">{translate("versions.noChanges")}</Text>
+ ) : (
+ <Table.Root striped>
+ <Table.Header>
+ <Table.Row>
+ <Table.ColumnHeader>{translate("versions.columns.path")}</Table.ColumnHeader>
+ <Table.ColumnHeader>{translate("versions.columns.operation")}</Table.ColumnHeader>
+ <Table.ColumnHeader>{translate("versions.columns.category")}</Table.ColumnHeader>
+ <Table.ColumnHeader>{translate("versions.columns.impact")}</Table.ColumnHeader>
+ <Table.ColumnHeader>{translate("versions.columns.occurrences")}</Table.ColumnHeader>
+ {valuesShown ? (
+ <>
+ <Table.ColumnHeader>{translate("versions.columns.before")}</Table.ColumnHeader>
+ <Table.ColumnHeader>{translate("versions.columns.after")}</Table.ColumnHeader>
+ </>
+ ) : undefined}
+ </Table.Row>
+ </Table.Header>
+ <Table.Body>
+ {diff.changes.map((change) => (
+ <Table.Row key={`${change.path}-${change.operation}`}>
+ <Table.Cell>
+ <Code fontSize="sm">{change.path}</Code>
+ </Table.Cell>
+ <Table.Cell>
+ {translate(`versions.operations.${change.operation}`, { defaultValue: change.operation })}
+ </Table.Cell>
+ <Table.Cell>
+ {translate(`versions.categories.${change.category}`, { defaultValue: change.category })}
+ </Table.Cell>
+ <Table.Cell>
+ <Badge colorPalette={IMPACT_COLORS[change.impact] ?? "gray"}>
+ {translate(`versions.impacts.${change.impact}`, { defaultValue: change.impact })}
+ </Badge>
+ </Table.Cell>
+ <Table.Cell>{change.occurrence_count}</Table.Cell>
+ {valuesShown ? (
+ <>
+ <Table.Cell>
+ <Code fontSize="sm">{renderValue(change.before_value)}</Code>
+ </Table.Cell>
+ <Table.Cell>
+ <Code fontSize="sm">{renderValue(change.after_value)}</Code>
+ </Table.Cell>
+ </>
+ ) : undefined}
+ </Table.Row>
+ ))}
+ </Table.Body>
+ </Table.Root>
+ )}
+
+ <Text color="fg.muted" fontSize="sm" mt={3}>
+ {translate("versions.observedStateBoundary")}
Review Comment:
This probably should go above the table, with an 'info' icon, something like
<img width="420" height="290" alt="Image" src="https://github.com/user-attachments/assets/c6dc09de-2ed9-4365-9103-5544c41b23c5" />
The reason is because if the table grows in size, the 'information' piece will be pushed down and missed for your users.
--
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 an API endpoint and UI tab comparing two stored Dag versions [airflow]
Posted by "pierrejeambrun (via GitHub)" <gi...@apache.org>.
pierrejeambrun commented on code in PR #73322:
URL: https://github.com/apache/airflow/pull/73322#discussion_r4104973101
##########
airflow-core/src/airflow/api_fastapi/core_api/routes/public/dag_versions.py:
##########
@@ -132,3 +138,76 @@ def get_dag_versions(
dag_versions=dag_versions,
total_entries=total_entries,
)
+
+
+@dag_versions_router.get(
+ "/{base_version_number}/diff/{target_version_number}",
+ responses=create_openapi_http_exception_doc(
+ [
+ status.HTTP_400_BAD_REQUEST,
+ status.HTTP_404_NOT_FOUND,
+ ]
+ ),
+ dependencies=[Depends(requires_access_dag(method="GET", access_entity=DagAccessEntity.VERSION))],
+ # Serializing unset fields as null would erase the absent-vs-null distinction the value
+ # fields document.
+ response_model_exclude_unset=True,
+)
+def get_dag_version_diff(
Review Comment:
Should this be public API or UI only at this point. I would put this in the UI for now so we can still update / break it while we stabilize the Rest API interface in the couple of next iteration. Then we can move it to public when we feel it's stable.
--
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 an API endpoint and UI tab comparing two stored Dag versions [airflow]
Posted by "ephraimbuddy (via GitHub)" <gi...@apache.org>.
ephraimbuddy commented on code in PR #73322:
URL: https://github.com/apache/airflow/pull/73322#discussion_r4108450220
##########
airflow-core/src/airflow/ui/src/pages/Dag/Versions/VersionDiff.tsx:
##########
@@ -0,0 +1,168 @@
+/*!
+ * 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 { Alert, Badge, Box, Code, Flex, Heading, Table, Text } from "@chakra-ui/react";
+import { useTranslation } from "react-i18next";
+
+import type { DagVersionDiffResponse } from "openapi/requests/types.gen";
+
+type VersionDiffProps = {
+ readonly baseVersionNumber: number;
+ readonly diff: DagVersionDiffResponse;
+ readonly targetVersionNumber: number;
+};
+
+const IMPACT_COLORS: Record<string, string> = {
+ authorization: "orange",
+ execution: "red",
+ metadata: "blue",
+ provenance: "gray",
+ unknown: "gray",
+};
+
+// A whole task added or removed carries its entire serialized payload, which would otherwise
+// unfold into one unreadable table cell.
+const MAX_VALUE_LENGTH = 120;
+
+const renderValue = (value: unknown) => {
+ if (value === undefined) {
+ return "—";
+ }
+
+ // A stored null is a value; only an absent side gets the dash above. An empty string is quoted
+ // so it reads as a value rather than as a blank cell.
+ const text = typeof value === "string" ? value || '""' : JSON.stringify(value);
+
+ return text.length > MAX_VALUE_LENGTH ? `${text.slice(0, MAX_VALUE_LENGTH)}…` : text;
+};
+
+export const VersionDiff = ({ baseVersionNumber, diff, targetVersionNumber }: VersionDiffProps) => {
+ const { t: translate } = useTranslation("dag");
+ const valuesShown = diff.values_status === "available";
+
+ if (diff.mode === "unavailable") {
+ return (
+ <Alert.Root status="info" title={translate("versions.unavailable.title")}>
+ <Alert.Description>
+ {diff.unavailable_reason === null || diff.unavailable_reason === undefined ? (
+ translate("versions.unavailable.withoutReason")
+ ) : (
+ <>
+ {translate("versions.unavailable.description")}{" "}
+ {/* A machine token, so it is shown as code rather than folded into a translated sentence. */}
+ <Code fontSize="sm">{diff.unavailable_reason}</Code>
+ </>
+ )}
+ </Alert.Description>
+ </Alert.Root>
+ );
+ }
+
+ return (
+ <Box>
+ <Flex alignItems="center" gap={3} justifyContent="space-between" mb={2}>
+ <Box>
+ <Heading size="md">
+ {translate("versions.heading", { base: baseVersionNumber, target: targetVersionNumber })}
+ </Heading>
+ <Text color="fg.muted" fontSize="sm">
+ {translate("versions.summary", {
+ baseSchema: diff.serialized_dag_schema_versions.base ?? "?",
+ count: diff.total_changes,
+ targetSchema: diff.serialized_dag_schema_versions.target ?? "?",
+ })}
+ </Text>
+ </Box>
+ <Flex alignItems="center" gap={2}>
+ <Badge colorPalette="teal">{translate("versions.observedState")}</Badge>
+ <Badge colorPalette={valuesShown ? "green" : "gray"}>
+ {translate(valuesShown ? "versions.valuesShown" : "versions.valuesHidden")}
+ </Badge>
+ {/* Says what grants values, since the viewer cannot grant it here: the server decides
+ from the caller's access to Dag code. */}
+ <Text color="fg.muted" fontSize="sm">
+ {translate("versions.codeAccess")}
+ </Text>
+ </Flex>
+ </Flex>
+
+ {diff.truncated ? (
+ <Alert.Root mb={2} status="warning" title={translate("versions.truncated.title")}>
+ <Alert.Description>{translate("versions.truncated.description")}</Alert.Description>
+ </Alert.Root>
+ ) : undefined}
+
+ {diff.changes.length === 0 ? (
+ <Text color="fg.muted">{translate("versions.noChanges")}</Text>
+ ) : (
+ <Table.Root striped>
+ <Table.Header>
+ <Table.Row>
+ <Table.ColumnHeader>{translate("versions.columns.path")}</Table.ColumnHeader>
+ <Table.ColumnHeader>{translate("versions.columns.operation")}</Table.ColumnHeader>
+ <Table.ColumnHeader>{translate("versions.columns.category")}</Table.ColumnHeader>
+ <Table.ColumnHeader>{translate("versions.columns.impact")}</Table.ColumnHeader>
+ <Table.ColumnHeader>{translate("versions.columns.occurrences")}</Table.ColumnHeader>
+ {valuesShown ? (
+ <>
+ <Table.ColumnHeader>{translate("versions.columns.before")}</Table.ColumnHeader>
+ <Table.ColumnHeader>{translate("versions.columns.after")}</Table.ColumnHeader>
+ </>
+ ) : undefined}
+ </Table.Row>
+ </Table.Header>
+ <Table.Body>
+ {diff.changes.map((change) => (
+ <Table.Row key={`${change.path}-${change.operation}`}>
+ <Table.Cell>
+ <Code fontSize="sm">{change.path}</Code>
+ </Table.Cell>
+ <Table.Cell>
+ {translate(`versions.operations.${change.operation}`, { defaultValue: change.operation })}
+ </Table.Cell>
+ <Table.Cell>
+ {translate(`versions.categories.${change.category}`, { defaultValue: change.category })}
+ </Table.Cell>
+ <Table.Cell>
+ <Badge colorPalette={IMPACT_COLORS[change.impact] ?? "gray"}>
+ {translate(`versions.impacts.${change.impact}`, { defaultValue: change.impact })}
+ </Badge>
+ </Table.Cell>
+ <Table.Cell>{change.occurrence_count}</Table.Cell>
+ {valuesShown ? (
+ <>
+ <Table.Cell>
+ <Code fontSize="sm">{renderValue(change.before_value)}</Code>
+ </Table.Cell>
+ <Table.Cell>
+ <Code fontSize="sm">{renderValue(change.after_value)}</Code>
+ </Table.Cell>
+ </>
+ ) : undefined}
+ </Table.Row>
+ ))}
+ </Table.Body>
+ </Table.Root>
+ )}
+
+ <Text color="fg.muted" fontSize="sm" mt={3}>
+ {translate("versions.observedStateBoundary")}
Review Comment:
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 an API endpoint and UI tab comparing two stored Dag versions [airflow]
Posted by "ephraimbuddy (via GitHub)" <gi...@apache.org>.
ephraimbuddy commented on PR #73322:
URL: https://github.com/apache/airflow/pull/73322#issuecomment-5838931615
Values Shown:
<img width="2560" height="900" alt="01-values-shown" src="https://github.com/user-attachments/assets/5b623413-a31e-4416-a648-00420f4847e9" />
Values Hidden:
<img width="2560" height="900" alt="02-values-hidden" src="https://github.com/user-attachments/assets/8d73902a-f1eb-486b-b066-d4a6448b3b80" />
Boundary tooltip
<img width="2560" height="900" alt="03-boundary-tooltip" src="https://github.com/user-attachments/assets/de075053-f2d8-4153-b6b6-1973d73e2344" />
Column Menu
<img width="2560" height="900" alt="04-column-menu" src="https://github.com/user-attachments/assets/4364651b-3c62-436d-b374-94c170c0ae85" />
version picker:
<img width="2560" height="900" alt="05-version-picker" src="https://github.com/user-attachments/assets/d5b57b37-e17b-4804-bb1c-9eb78695e764" />
--
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 an API endpoint and UI tab comparing two stored Dag versions [airflow]
Posted by "ephraimbuddy (via GitHub)" <gi...@apache.org>.
ephraimbuddy commented on code in PR #73322:
URL: https://github.com/apache/airflow/pull/73322#discussion_r4105261575
##########
airflow-core/src/airflow/api_fastapi/core_api/routes/public/dag_versions.py:
##########
@@ -132,3 +138,76 @@ def get_dag_versions(
dag_versions=dag_versions,
total_entries=total_entries,
)
+
+
+@dag_versions_router.get(
+ "/{base_version_number}/diff/{target_version_number}",
+ responses=create_openapi_http_exception_doc(
+ [
+ status.HTTP_400_BAD_REQUEST,
+ status.HTTP_404_NOT_FOUND,
+ ]
+ ),
+ dependencies=[Depends(requires_access_dag(method="GET", access_entity=DagAccessEntity.VERSION))],
+ # Serializing unset fields as null would erase the absent-vs-null distinction the value
+ # fields document.
+ response_model_exclude_unset=True,
+)
+def get_dag_version_diff(
Review Comment:
I will add experimental to the endpoint so we can vary it later without 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 an API endpoint and UI tab comparing two stored Dag versions [airflow]
Posted by "ephraimbuddy (via GitHub)" <gi...@apache.org>.
ephraimbuddy commented on code in PR #73322:
URL: https://github.com/apache/airflow/pull/73322#discussion_r4108447714
##########
airflow-core/src/airflow/api_fastapi/core_api/routes/public/dag_versions.py:
##########
@@ -132,3 +138,76 @@ def get_dag_versions(
dag_versions=dag_versions,
total_entries=total_entries,
)
+
+
+@dag_versions_router.get(
+ "/{base_version_number}/diff/{target_version_number}",
Review Comment:
makes sense. 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 an API endpoint and UI tab comparing two stored Dag versions [airflow]
Posted by "ephraimbuddy (via GitHub)" <gi...@apache.org>.
ephraimbuddy merged PR #73322:
URL: https://github.com/apache/airflow/pull/73322
--
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 an API endpoint and UI tab comparing two stored Dag versions [airflow]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #73322:
URL: https://github.com/apache/airflow/pull/73322#issuecomment-5981982388
### Backport failed to create: airflow-ctl/v0-1-test. View the failure log <a href='https://github.com/apache/airflow/actions/runs/37216002594'> Run details </a>
Note: As of [Merging PRs targeted for Airflow 3.X](https://github.com/apache/airflow/blob/main/dev/README_AIRFLOW3_DEV.md#merging-prs-targeted-for-airflow-3x)
the committer who merges the PR is responsible for backporting the PRs that are bug fixes (generally speaking) to the maintenance branches.
In matter of doubt please ask in [#release-management](https://apache-airflow.slack.com/archives/C03G9H97MM2) Slack channel.
<table>
<tr>
<th>Status</th>
<th>Branch</th>
<th>Result</th>
</tr>
<tr>
<td>❌</td>
<td>airflow-ctl/v0-1-test</td>
<td><a href="https://github.com/apache/airflow/commit/b58cfa912a92a26507889a58cb3a34cd0a6dd0a6"><img src='https://img.shields.io/badge/Commit-b58cfa9-red' alt='Commit Link'></a></td>
</tr>
</table>
You can attempt to backport this manually by running:
```bash
cherry_picker b58cfa9 airflow-ctl/v0-1-test
```
This should apply the commit to the airflow-ctl/v0-1-test branch and leave the commit in conflict state marking
the files that need manual conflict resolution.
After you have resolved the conflicts, you can continue the backport process by running:
```bash
cherry_picker --continue
```
If you don't have cherry-picker installed, see the [installation guide](https://github.com/apache/airflow/blob/main/dev/README.md#how-to-backport-pr-with-cherry-picker-cli).
--
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