-
Notifications
You must be signed in to change notification settings - Fork 16.5k
Feat: Add version change indicators for Dag and bundle versions in Grid view #53216
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
f48e7ec
25ccdae
cf678a8
5a59df4
fdfccb9
8d3d862
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -22,7 +22,7 @@ | |
|
|
||
| import structlog | ||
| from fastapi import Depends, HTTPException, status | ||
| from sqlalchemy import select | ||
| from sqlalchemy import func, select | ||
| from sqlalchemy.orm import joinedload | ||
|
|
||
| from airflow.api_fastapi.auth.managers.models.resource_details import DagAccessEntity | ||
|
|
@@ -274,17 +274,34 @@ def get_grid_runs( | |
| triggering_user: QueryDagRunTriggeringUserSearch, | ||
| ) -> list[GridRunsResponse]: | ||
| """Get info about a run for the grid.""" | ||
| # Retrieve, sort the previous DAG Runs | ||
| base_query = select( | ||
| DagRun.dag_id, | ||
| DagRun.run_id, | ||
| DagRun.queued_at, | ||
| DagRun.start_date, | ||
| DagRun.end_date, | ||
| DagRun.run_after, | ||
| DagRun.state, | ||
| DagRun.run_type, | ||
| ).where(DagRun.dag_id == dag_id) | ||
| # get the highest dag_version_number from TIs for each run | ||
| latest_ti_version = ( | ||
| select( | ||
| TaskInstance.run_id, | ||
| func.max(DagVersion.version_number).label("version_number"), | ||
| ) | ||
| .join(DagVersion, TaskInstance.dag_version_id == DagVersion.id) | ||
| .where(TaskInstance.dag_id == dag_id) | ||
| .group_by(TaskInstance.run_id) | ||
| .subquery() | ||
| ) | ||
|
Comment on lines
+277
to
+287
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We don't need to go through all that trouble to only retrieve the 'last' version. Just return the Also this will be consistent for other contributor. People will get confused as to why a DagRun has I find this confusing and it adds a lot of unecessary application code. |
||
|
|
||
| base_query = ( | ||
| select( | ||
| DagRun.dag_id, | ||
| DagRun.run_id, | ||
| DagRun.queued_at, | ||
| DagRun.start_date, | ||
| DagRun.end_date, | ||
| DagRun.run_after, | ||
| DagRun.state, | ||
| DagRun.run_type, | ||
| DagRun.bundle_version, | ||
| latest_ti_version.c.version_number.label("dag_version_number"), | ||
| ) | ||
| .outerjoin(latest_ti_version, DagRun.run_id == latest_ti_version.c.run_id) | ||
| .where(DagRun.dag_id == dag_id) | ||
| ) | ||
|
|
||
| # This comparison is to fall back to DAG timetable when no order_by is provided | ||
| if order_by.value == [order_by.get_primary_key_string()]: | ||
|
|
@@ -355,7 +372,9 @@ def get_grid_ti_summaries( | |
| TaskInstance.dag_version_id, | ||
| TaskInstance.start_date, | ||
| TaskInstance.end_date, | ||
| DagVersion.version_number, | ||
| ) | ||
| .outerjoin(DagVersion, TaskInstance.dag_version_id == DagVersion.id) | ||
| .where(TaskInstance.dag_id == dag_id) | ||
| .where( | ||
| TaskInstance.run_id == run_id, | ||
|
|
@@ -378,6 +397,7 @@ def get_grid_ti_summaries( | |
| "state": ti.state, | ||
| "start_date": ti.start_date, | ||
| "end_date": ti.end_date, | ||
| "dag_version_number": ti.version_number, | ||
| } | ||
| ) | ||
| serdag = _get_serdag( | ||
|
|
||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If we only use this in the grid view then we should move it to |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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 { Box } from "@chakra-ui/react"; | ||
| import { useTranslation } from "react-i18next"; | ||
| import { FiGitCommit } from "react-icons/fi"; | ||
|
|
||
| import { Tooltip } from "src/components/ui"; | ||
|
|
||
| type BundleVersionIndicatorProps = { | ||
| readonly bundleVersion: string | undefined; | ||
| }; | ||
|
|
||
| export const BundleVersionIndicator = ({ bundleVersion }: BundleVersionIndicatorProps) => { | ||
| const { t: translate } = useTranslation("components"); | ||
|
|
||
| return ( | ||
| <Tooltip content={`${translate("versionDetails.bundleVersion")}: ${bundleVersion}`}> | ||
| <Box color="orange.focusRing" left={-2} position="absolute" top={93} zIndex={1}> | ||
| <FiGitCommit size={15} /> | ||
| </Box> | ||
| </Tooltip> | ||
| ); | ||
| }; | ||
|
|
||
| const CONTAINER_STYLES = { | ||
| horizontal: { | ||
| height: 0.5, | ||
| left: "50%", | ||
| top: 0, | ||
| transform: "translate(-50%, -50%)", | ||
| width: 4.5, | ||
| }, | ||
| vertical: { | ||
| height: 104, | ||
| left: -1.25, | ||
| top: -1.5, | ||
| width: 0.5, | ||
| }, | ||
| } as const; | ||
|
|
||
| const CIRCLE_STYLES = { | ||
| horizontal: { | ||
| height: 1.5, | ||
| left: "50%", | ||
| top: "50%", | ||
| transform: "translate(-50%, -50%)", | ||
| width: 1.5, | ||
| }, | ||
| vertical: { | ||
| height: 1.5, | ||
| left: "50%", | ||
| top: -1, | ||
| transform: "translateX(-50%)", | ||
| width: 1.5, | ||
| }, | ||
| } as const; | ||
|
|
||
| type DagVersionIndicatorProps = { | ||
| readonly dagVersionNumber: number | undefined; | ||
| readonly orientation?: "horizontal" | "vertical"; | ||
| }; | ||
|
|
||
| export const DagVersionIndicator = ({ | ||
| dagVersionNumber, | ||
| orientation = "vertical", | ||
| }: DagVersionIndicatorProps) => { | ||
| const isVertical = orientation === "vertical"; | ||
| const currentContainerStyle = CONTAINER_STYLES[orientation]; | ||
| const currentCircleStyle = CIRCLE_STYLES[orientation]; | ||
|
|
||
| return ( | ||
| <Box | ||
| aria-label={`Version ${dagVersionNumber} indicator`} | ||
| as="output" | ||
| position="absolute" | ||
| zIndex={1} | ||
| {...currentContainerStyle} | ||
| > | ||
| <Box | ||
| bg="orange.focusRing" | ||
| height={isVertical ? "full" : 0.5} | ||
| position="absolute" | ||
| width={isVertical ? 0.5 : "full"} | ||
| /> | ||
|
|
||
| <Tooltip | ||
| content={`v${dagVersionNumber ?? ""}`} | ||
| positioning={{ | ||
| placement: isVertical ? "top" : "right", | ||
| }} | ||
| > | ||
| <Box | ||
| _hover={{ | ||
| cursor: "pointer", | ||
| transform: `${currentCircleStyle.transform} scale(1.2)`, | ||
| }} | ||
| bg="orange.focusRing" | ||
| borderRadius="full" | ||
| position="absolute" | ||
| transition="all 0.2s ease-in-out" | ||
| {...currentCircleStyle} | ||
| /> | ||
| </Tooltip> | ||
| </Box> | ||
| ); | ||
| }; |
| Original file line number | Diff line number | Diff line change | ||||||||
|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,46 @@ | ||||||||||
| /*! | ||||||||||
| * 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 { createListCollection } from "@chakra-ui/react"; | ||||||||||
|
|
||||||||||
| export enum VersionIndicatorDisplayOptions { | ||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
We use this a lot. Let's try to have a simpler variable name. |
||||||||||
| ALL = "all", | ||||||||||
| BUNDLE = "bundle", | ||||||||||
| DAG = "dag", | ||||||||||
|
Comment on lines
+23
to
+24
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nit:
Suggested change
|
||||||||||
| NONE = "none", | ||||||||||
| } | ||||||||||
|
|
||||||||||
| const validOptions = new Set<string>(Object.values(VersionIndicatorDisplayOptions)); | ||||||||||
|
|
||||||||||
| export const isVersionIndicatorDisplayOption = (value: unknown): value is VersionIndicatorDisplayOptions => | ||||||||||
| typeof value === "string" && validOptions.has(value); | ||||||||||
|
|
||||||||||
| export const showVersionIndicatorOptions = createListCollection({ | ||||||||||
| items: [ | ||||||||||
| { label: "dag:panel.showVersionIndicator.options.showAll", value: VersionIndicatorDisplayOptions.ALL }, | ||||||||||
| { | ||||||||||
| label: "dag:panel.showVersionIndicator.options.showBundleVersion", | ||||||||||
| value: VersionIndicatorDisplayOptions.BUNDLE, | ||||||||||
| }, | ||||||||||
| { | ||||||||||
| label: "dag:panel.showVersionIndicator.options.showDagVersion", | ||||||||||
| value: VersionIndicatorDisplayOptions.DAG, | ||||||||||
| }, | ||||||||||
| { label: "dag:panel.showVersionIndicator.options.hideAll", value: VersionIndicatorDisplayOptions.NONE }, | ||||||||||
| ], | ||||||||||
| }); | ||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is
bundle_versionused?