You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by "davidfgcorreia (via GitHub)" <gi...@apache.org> on 2025/06/13 09:59:00 UTC
[PR] feat(api/ui)#50779 : add asset group filtering and asset group views [airflow]
davidfgcorreia opened a new pull request, #51682:
URL: https://github.com/apache/airflow/pull/51682
Add backend and OpenAPI support for filtering assets by group (`group_pattern`) in /assets endpoint.
Implement multi-asset dependency queries in /ui/dependencies endpoint with node_ids parameter.
Add new frontend pages and components for asset group views, including group graph and sidebar.
Refactor asset list and group list to support group filtering, merging, and deduplication.
Add and update tests for group filtering, group views, and multi-asset dependency queries.
introduced small bug in caching on the asset_grath.
https://youtu.be/ybpRUjEhdoQ
<!--
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.
-->
<!--
Thank you for contributing! Please make sure that your code changes
are covered with tests. And in case of new features or big changes
remember to adjust the documentation.
Feel free to ping committers for the review!
In case of an existing issue, reference it using one of the following:
closes: #ISSUE
related: #50779
How to write a good git commit message:
http://chris.beams.io/posts/git-commit/
-->
<!-- Please keep an empty line above the dashes. -->
---
**^ Add meaningful description above**
Read the **[Pull Request Guidelines](https://github.com/apache/airflow/blob/main/contributing-docs/05_pull_requests.rst#pull-request-guidelines)** for more information.
In case of fundamental code changes, an Airflow Improvement Proposal ([AIP](https://cwiki.apache.org/confluence/display/AIRFLOW/Airflow+Improvement+Proposals)) is needed.
In case of a new dependency, check compliance with the [ASF 3rd Party License Policy](https://www.apache.org/legal/resolved.html#category-x).
In case of backwards incompatible changes please leave a note in a newsfragment file, named `{pr_number}.significant.rst` or `{issue_number}.significant.rst`, in [airflow-core/newsfragments](https://github.com/apache/airflow/tree/main/airflow-core/newsfragments).
--
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] feat(api/ui)#50779 : add asset group filtering and asset group views [airflow]
Posted by "pierrejeambrun (via GitHub)" <gi...@apache.org>.
pierrejeambrun closed pull request #51682: feat(api/ui)#50779 : add asset group filtering and asset group views
URL: https://github.com/apache/airflow/pull/51682
--
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] feat(api/ui)#50779 : add asset group filtering and asset group views [airflow]
Posted by "pierrejeambrun (via GitHub)" <gi...@apache.org>.
pierrejeambrun commented on code in PR #51682:
URL: https://github.com/apache/airflow/pull/51682#discussion_r2263049532
##########
airflow-core/src/airflow/api_fastapi/core_api/routes/ui/dependencies.py:
##########
@@ -41,12 +42,27 @@
),
dependencies=[Depends(requires_access_dag("GET", DagAccessEntity.DEPENDENCIES))],
)
-def get_dependencies(session: SessionDep, node_id: str | None = None) -> BaseGraphResponse:
- """Dependencies graph."""
+def get_dependencies(
+ node_id: str | None = None,
+ node_ids: list[str] = Query(None, description="List of node ids"),
+) -> BaseGraphResponse:
+ """Dependencies graph. Supports a single node_id or multiple node_ids as exploded query parameters (node_ids=foo&node_ids=bar)."""
Review Comment:
```suggestion
"""Dependencies graph. Supports a single node_id or multiple node_ids
```
##########
airflow-core/src/airflow/api_fastapi/core_api/routes/ui/dependencies.py:
##########
@@ -41,12 +42,27 @@
),
dependencies=[Depends(requires_access_dag("GET", DagAccessEntity.DEPENDENCIES))],
)
-def get_dependencies(session: SessionDep, node_id: str | None = None) -> BaseGraphResponse:
- """Dependencies graph."""
+def get_dependencies(
+ node_id: str | None = None,
Review Comment:
```suggestion
```
We should probably only use `node_ids` even for single node to simplify the API. That's private API so it doens't have to be backward compatible.
##########
airflow-core/src/airflow/ui/src/vite-env.d.ts:
##########
@@ -19,6 +19,7 @@
*/
/// <reference types="vite/client" />
+/// <reference types="vite/types/importMeta.d.ts" />
Review Comment:
I don't think it's needed anymore. That must have been fixed in other PRs.
##########
airflow-core/src/airflow/api_fastapi/core_api/routes/public/assets.py:
##########
@@ -125,7 +127,7 @@ def depends(cls, only_active: bool = True) -> OnlyActiveFilter:
def get_assets(
limit: QueryLimit,
offset: QueryOffset,
- name_pattern: QueryAssetNamePatternSearch,
+ asset_pattern_filter: QueryAssetPatternFilter,
Review Comment:
We shouldn't replace like this, but add the `group_pattern_filter` next to it, so we can chose what to filter on.
##########
airflow-core/src/airflow/api_fastapi/core_api/routes/public/assets.py:
##########
@@ -206,6 +208,88 @@ def get_assets(
)
+@assets_router.get(
+ "/assets/groups",
+ responses=create_openapi_http_exception_doc([status.HTTP_404_NOT_FOUND]),
+ dependencies=[
+ Depends(requires_access_asset(method="GET")),
+ ],
+)
+def get_asset_groups(
+ limit: QueryLimit,
+ offset: QueryOffset,
+ only_active: Annotated[OnlyActiveFilter, Depends(OnlyActiveFilter.depends)],
+ session: SessionDep,
+ order_by: Annotated[str, Query(alias="order_by")] = "group",
+ group: str | None = Query(default=None, description="Filter by grupo"),
+) -> AssetGroupCollectionResponse:
+ """Get asset groups."""
+ offset_value = offset.value if offset.value is not None else 0
+
+ group_query = (
+ select(AssetModel.group, func.count(AssetModel.id))
+ .where(
+ AssetModel.active.has() if only_active.value else True,
+ *([AssetModel.group == group] if group else []),
+ )
+ .group_by(AssetModel.group)
+ )
+ group_rows = session.execute(group_query).all()
+ limit_value = limit.value if limit.value is not None else len(group_rows)
+ # Ordene em Python
+ if order_by == "count":
+ group_rows.sort(key=lambda x: (x[1], x[0]))
+ elif order_by == "-count":
+ group_rows.sort(key=lambda x: (-x[1], x[0]))
+ elif order_by == "-group":
+ group_rows.sort(key=lambda x: x[0], reverse=True)
+ else:
+ group_rows.sort(key=lambda x: x[0])
+ # Aplique paginação em Python
+ paginated_rows = group_rows[offset_value : offset_value + limit_value]
+ groups = [row[0] for row in paginated_rows]
+ group_counts = {row[0]: row[1] for row in paginated_rows}
+
+ # Query all assets for these groups
+ assets_query = (
+ select(AssetModel)
+ .where(AssetModel.group.in_(groups))
+ .options(
+ subqueryload(AssetModel.scheduled_dags),
+ subqueryload(AssetModel.producing_tasks),
+ subqueryload(AssetModel.consuming_tasks),
+ )
+ )
+ assets = session.scalars(assets_query).all()
Review Comment:
This will query all assets from the db. Can't happen I think
##########
airflow-core/src/airflow/ui/src/pages/Asset/AssetGroupSidebar.tsx:
##########
@@ -0,0 +1,97 @@
+/*!
+ * 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, Text, HStack, VStack, Badge, Button } from "@chakra-ui/react";
+import { useTranslation } from "react-i18next";
+
+import type { AssetResponse } from "openapi/requests/types.gen";
+
+type AssetGroupSidebarProps = {
+ assets: Array<AssetResponse>;
+ groupName: string;
+};
+
+export const AssetGroupSidebar: React.FC<AssetGroupSidebarProps> = ({ assets, groupName }) => {
+ const { t: translate } = useTranslation(["assets", "common"]);
+
+ const producingTasks = assets.reduce((acc, asset) => acc + asset.producing_tasks.length, 0);
+ const consumingDags = assets.reduce((acc, asset) => acc + asset.consuming_tasks.length, 0);
+
+ // Collect all last asset events from all assets
+ const events = assets
+ .map((asset) => asset.last_asset_event)
+ .filter((ev): ev is NonNullable<typeof ev> => Boolean(ev));
+
+ return (
+ <VStack align="stretch" gap={4}>
+ <Box bg="chakra-subtle-bg" borderColor="chakra-border-color" borderRadius="md" borderWidth={1} p={4}>
+ <HStack gap={2} mb={2}>
+ <Badge aria-label={translate("group", { ns: "assets" })} colorScheme="gray" fontSize="lg">
+ {translate("databaseIcon", { ns: "assets" })}
+ </Badge>
+ <Heading size="md">{groupName}</Heading>
+ </HStack>
+ <Text color="chakra-fg" fontSize="sm" mb={2}>
+ <b>{translate("group", { ns: "assets" })}</b>
+ <br />
+ {groupName}
+ </Text>
+ <HStack gap={4} mb={2}>
+ <Box>
+ <Text color="chakra-fg" fontSize="xs">
+ {translate("producingTasks", { ns: "assets" })}
+ </Text>
+ <Button disabled size="sm" variant="outline">
+ {producingTasks} {translate("task_one", { count: producingTasks, ns: "common" })}
+ </Button>
+ </Box>
+ <Box>
+ <Text color="chakra-fg" fontSize="xs">
+ {translate("consumingDags", { ns: "assets" })}
+ </Text>
+ <Button disabled size="sm" variant="outline">
+ {consumingDags} {translate("dag_one", { count: consumingDags, ns: "common" })}
+ </Button>
+ </Box>
+ </HStack>
+ </Box>
+ <Box bg="chakra-subtle-bg" borderColor="chakra-border-color" borderRadius="md" borderWidth={1} p={4}>
+ <HStack mb={2}>
+ <Badge colorScheme="blue" />
+ <Heading size="sm">{translate("lastAssetEvent", { ns: "assets" })}</Heading>
+ </HStack>
+ {events.length === 0 ? (
+ <Text color="chakra-fg-subtle" fontSize="sm">
+ {translate("noAssetEvents", { ns: "dashboard" })}
+ </Text>
+ ) : (
+ events.map((ev) => (
+ <Box bg="chakra-subtle-bg-hover" borderRadius="md" key={ev.id} mb={2} p={2}>
+ <Text color="chakra-fg-subtle" fontSize="xs">
+ {ev.timestamp}
+ </Text>
Review Comment:
We probably shouldn't display timestamp as text, check other location of the app for `Time` component.
##########
airflow-core/src/airflow/ui/src/pages/AssetsList/AssetsList.tsx:
##########
@@ -92,64 +105,80 @@ const createColumns = (translate: (key: string) => string): Array<ColumnDef<Asse
},
];
-const NAME_PATTERN_PARAM = SearchParamsKeys.NAME_PATTERN;
-
export const AssetsList = () => {
const { t: translate } = useTranslation(["assets", "common"]);
const [searchParams, setSearchParams] = useSearchParams();
- const [namePattern, setNamePattern] = useState(searchParams.get(NAME_PATTERN_PARAM) ?? undefined);
+ const [searchValue, setSearchValue] = useState(
+ searchParams.get(SearchParamsKeys.NAME_PATTERN) ??
+ searchParams.get(SearchParamsKeys.GROUP_PATTERN) ??
+ undefined,
+ );
const { setTableURLState, tableURLState } = useTableURLState();
const { pagination, sorting } = tableURLState;
const [sort] = sorting;
const orderBy = sort ? `${sort.desc ? "-" : ""}${sort.id}` : undefined;
+ const searchMatchMode = "any"; // "any" para OR, "all" para AND
Review Comment:
comment not in english.
##########
airflow-core/src/airflow/ui/src/queries/useMultiDependencyGraph.ts:
##########
@@ -0,0 +1,53 @@
+/*!
+ * 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 { useQueryClient, type UseQueryOptions } from "@tanstack/react-query";
+
+import {
+ useDependenciesServiceGetDependencies,
+ UseDependenciesServiceGetDependenciesKeyFn,
+} from "openapi/queries";
+import type { BaseGraphResponse } from "openapi/requests/types.gen";
+
+/**
+ * Hook to fetch the dependency graph for a list of asset ids.
+ * Uses the backend endpoint that accepts node_ids as an exploded array (node_ids=foo&node_ids=bar).
+ * Caches the result by ids.
+ */
+export const useMultiDependencyGraph = (
+ assetIds: Array<string>,
+ options?: Omit<UseQueryOptions<BaseGraphResponse, unknown>, "queryFn" | "queryKey">,
+) => {
+ const queryClient = useQueryClient();
+
+ // Always send nodeIds as an array of asset:... strings
+ const nodeIdsParam = assetIds.map((id) => (id.startsWith("asset:") ? id : `asset:${id}`));
+
+ const query = useDependenciesServiceGetDependencies({ nodeIds: nodeIdsParam }, undefined, options);
+
+ if (query.data) {
+ const key = UseDependenciesServiceGetDependenciesKeyFn({ nodeIds: nodeIdsParam });
+ const queryData = queryClient.getQueryData(key);
+
+ if (!Boolean(queryData)) {
+ queryClient.setQueryData(key, query.data);
+ }
+ }
+
+ return query;
+};
Review Comment:
That should `useDependencyGraph.ts` we don't need another hook doing the exact same thing.
##########
airflow-core/src/airflow/ui/src/pages/AssetsGroupedList/AssetsGroupedList.tsx:
##########
@@ -0,0 +1,282 @@
+/*!
+ * 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, Link, IconButton, VStack, Flex, Heading, Text, Input } from "@chakra-ui/react";
+import { useState, useMemo } from "react";
+import { useTranslation } from "react-i18next";
+import { FaChevronDown, FaChevronRight, FaChevronUp } from "react-icons/fa";
+import { Link as RouterLink } from "react-router-dom";
+
+import { useAssetServiceGetAssetGroups } from "openapi/queries";
+import type { AssetResponse } from "openapi/requests/types.gen";
+import { DataTable } from "src/components/DataTable";
+import { useTableURLState } from "src/components/DataTable/useTableUrlState";
+import { ErrorAlert } from "src/components/ErrorAlert";
+import Time from "src/components/Time";
+
+import { CreateAssetEvent } from "../Asset/CreateAssetEvent";
+import { DependencyPopover } from "../AssetsList/DependencyPopover";
+
+const NameCell = ({ original }: { readonly original: AssetResponse }) => (
+ <Link asChild color="fg.info" fontWeight="bold">
+ <RouterLink to={`/assets/${original.id}`}>{original.name}</RouterLink>
+ </Link>
+);
+
+const LastAssetEventCell = ({ original }: { readonly original: AssetResponse }) => {
+ const assetEvent = original.last_asset_event;
+ const timestamp = assetEvent?.timestamp;
+
+ if (timestamp === undefined || timestamp === null) {
+ return undefined;
+ }
+
+ return <Time datetime={timestamp} />;
+};
+
+const GroupCell = ({ original }: { readonly original: AssetResponse }) => {
+ const { group } = original;
+
+ if (!group) {
+ return undefined;
+ }
+
+ return (
+ <Link asChild color="fg.info" fontWeight="bold">
+ <RouterLink to={`/assets/group/${group}`}>{group}</RouterLink>
+ </Link>
+ );
+};
+
+const ConsumingDagsCell = ({ original }: { readonly original: AssetResponse }) =>
+ original.consuming_tasks.length ? (
+ <DependencyPopover dependencies={original.consuming_tasks} type="Dag" />
+ ) : undefined;
+
+const ProducingTasksCell = ({ original }: { readonly original: AssetResponse }) =>
+ original.producing_tasks.length ? (
+ <DependencyPopover dependencies={original.producing_tasks} type="Task" />
+ ) : undefined;
+
+const TriggerCell = ({ original }: { readonly original: AssetResponse }) => (
+ <CreateAssetEvent asset={original} withText={false} />
+);
+
+const truncate = (str: string, max = 32) => (str.length > max ? `${str.slice(0, max - 3)}...` : str);
+
+const nameCellRenderer = ({ row: { original } }: { row: { original: AssetResponse } }) => (
+ <NameCell original={original} />
+);
+const lastAssetEventCellRenderer = ({ row: { original } }: { row: { original: AssetResponse } }) => (
+ <LastAssetEventCell original={original} />
+);
+const groupCellRenderer = ({ row: { original } }: { row: { original: AssetResponse } }) => (
+ <GroupCell original={original} />
+);
+const consumingDagsCellRenderer = ({ row: { original } }: { row: { original: AssetResponse } }) => (
+ <ConsumingDagsCell original={original} />
+);
+const producingTasksCellRenderer = ({ row: { original } }: { row: { original: AssetResponse } }) => (
+ <ProducingTasksCell original={original} />
+);
+const triggerCellRenderer = ({ row: { original } }: { row: { original: AssetResponse } }) => (
+ <TriggerCell original={original} />
+);
+
+export const AssetsGroupedList = () => {
+ const { t: translate } = useTranslation(["assets", "common"]);
+ const { setTableURLState, tableURLState } = useTableURLState();
+ const { pagination, sorting } = tableURLState;
+ const [sort] = sorting;
+ const orderBy = sort ? `${sort.desc ? "-" : ""}${sort.id}` : undefined;
+
+ // Use o novo hook para buscar grupos paginados
Review Comment:
Comment not in english.
##########
airflow-core/src/airflow/ui/src/pages/AssetsGroupedList/AssetsGroupedList.tsx:
##########
@@ -0,0 +1,282 @@
+/*!
+ * 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, Link, IconButton, VStack, Flex, Heading, Text, Input } from "@chakra-ui/react";
+import { useState, useMemo } from "react";
+import { useTranslation } from "react-i18next";
+import { FaChevronDown, FaChevronRight, FaChevronUp } from "react-icons/fa";
+import { Link as RouterLink } from "react-router-dom";
+
+import { useAssetServiceGetAssetGroups } from "openapi/queries";
+import type { AssetResponse } from "openapi/requests/types.gen";
+import { DataTable } from "src/components/DataTable";
+import { useTableURLState } from "src/components/DataTable/useTableUrlState";
+import { ErrorAlert } from "src/components/ErrorAlert";
+import Time from "src/components/Time";
+
+import { CreateAssetEvent } from "../Asset/CreateAssetEvent";
+import { DependencyPopover } from "../AssetsList/DependencyPopover";
+
+const NameCell = ({ original }: { readonly original: AssetResponse }) => (
+ <Link asChild color="fg.info" fontWeight="bold">
+ <RouterLink to={`/assets/${original.id}`}>{original.name}</RouterLink>
+ </Link>
+);
+
+const LastAssetEventCell = ({ original }: { readonly original: AssetResponse }) => {
+ const assetEvent = original.last_asset_event;
+ const timestamp = assetEvent?.timestamp;
+
+ if (timestamp === undefined || timestamp === null) {
+ return undefined;
+ }
+
+ return <Time datetime={timestamp} />;
+};
+
+const GroupCell = ({ original }: { readonly original: AssetResponse }) => {
+ const { group } = original;
+
+ if (!group) {
+ return undefined;
+ }
+
+ return (
+ <Link asChild color="fg.info" fontWeight="bold">
+ <RouterLink to={`/assets/group/${group}`}>{group}</RouterLink>
+ </Link>
+ );
+};
+
+const ConsumingDagsCell = ({ original }: { readonly original: AssetResponse }) =>
+ original.consuming_tasks.length ? (
+ <DependencyPopover dependencies={original.consuming_tasks} type="Dag" />
+ ) : undefined;
+
+const ProducingTasksCell = ({ original }: { readonly original: AssetResponse }) =>
+ original.producing_tasks.length ? (
+ <DependencyPopover dependencies={original.producing_tasks} type="Task" />
+ ) : undefined;
+
+const TriggerCell = ({ original }: { readonly original: AssetResponse }) => (
+ <CreateAssetEvent asset={original} withText={false} />
+);
+
+const truncate = (str: string, max = 32) => (str.length > max ? `${str.slice(0, max - 3)}...` : str);
+
+const nameCellRenderer = ({ row: { original } }: { row: { original: AssetResponse } }) => (
+ <NameCell original={original} />
+);
+const lastAssetEventCellRenderer = ({ row: { original } }: { row: { original: AssetResponse } }) => (
+ <LastAssetEventCell original={original} />
+);
+const groupCellRenderer = ({ row: { original } }: { row: { original: AssetResponse } }) => (
+ <GroupCell original={original} />
+);
+const consumingDagsCellRenderer = ({ row: { original } }: { row: { original: AssetResponse } }) => (
+ <ConsumingDagsCell original={original} />
+);
+const producingTasksCellRenderer = ({ row: { original } }: { row: { original: AssetResponse } }) => (
+ <ProducingTasksCell original={original} />
+);
+const triggerCellRenderer = ({ row: { original } }: { row: { original: AssetResponse } }) => (
+ <TriggerCell original={original} />
+);
+
Review Comment:
Similarly to other columns definition, inline this directly in the columns definition.
##########
airflow-core/src/airflow/ui/src/queries/useMultiDependencyGraph.ts:
##########
@@ -0,0 +1,53 @@
+/*!
+ * 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 { useQueryClient, type UseQueryOptions } from "@tanstack/react-query";
+
+import {
+ useDependenciesServiceGetDependencies,
+ UseDependenciesServiceGetDependenciesKeyFn,
+} from "openapi/queries";
+import type { BaseGraphResponse } from "openapi/requests/types.gen";
+
+/**
+ * Hook to fetch the dependency graph for a list of asset ids.
+ * Uses the backend endpoint that accepts node_ids as an exploded array (node_ids=foo&node_ids=bar).
+ * Caches the result by ids.
+ */
Review Comment:
```suggestion
```
##########
airflow-core/src/airflow/ui/src/queries/useMultiDependencyGraph.ts:
##########
@@ -0,0 +1,53 @@
+/*!
+ * 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 { useQueryClient, type UseQueryOptions } from "@tanstack/react-query";
+
+import {
+ useDependenciesServiceGetDependencies,
+ UseDependenciesServiceGetDependenciesKeyFn,
+} from "openapi/queries";
+import type { BaseGraphResponse } from "openapi/requests/types.gen";
+
+/**
+ * Hook to fetch the dependency graph for a list of asset ids.
+ * Uses the backend endpoint that accepts node_ids as an exploded array (node_ids=foo&node_ids=bar).
+ * Caches the result by ids.
+ */
+export const useMultiDependencyGraph = (
+ assetIds: Array<string>,
+ options?: Omit<UseQueryOptions<BaseGraphResponse, unknown>, "queryFn" | "queryKey">,
+) => {
+ const queryClient = useQueryClient();
+
+ // Always send nodeIds as an array of asset:... strings
+ const nodeIdsParam = assetIds.map((id) => (id.startsWith("asset:") ? id : `asset:${id}`));
+
+ const query = useDependenciesServiceGetDependencies({ nodeIds: nodeIdsParam }, undefined, options);
+
+ if (query.data) {
+ const key = UseDependenciesServiceGetDependenciesKeyFn({ nodeIds: nodeIdsParam });
+ const queryData = queryClient.getQueryData(key);
+
+ if (!Boolean(queryData)) {
+ queryClient.setQueryData(key, query.data);
+ }
Review Comment:
This is done automatically by react query. Since you are setting the cache for `nodeIdsParam` which is one of the query parameters. That is not needed.
##########
airflow-core/src/airflow/api_fastapi/core_api/routes/public/assets.py:
##########
@@ -206,6 +208,88 @@ def get_assets(
)
+@assets_router.get(
+ "/assets/groups",
+ responses=create_openapi_http_exception_doc([status.HTTP_404_NOT_FOUND]),
+ dependencies=[
+ Depends(requires_access_asset(method="GET")),
+ ],
+)
+def get_asset_groups(
+ limit: QueryLimit,
+ offset: QueryOffset,
+ only_active: Annotated[OnlyActiveFilter, Depends(OnlyActiveFilter.depends)],
+ session: SessionDep,
+ order_by: Annotated[str, Query(alias="order_by")] = "group",
+ group: str | None = Query(default=None, description="Filter by grupo"),
+) -> AssetGroupCollectionResponse:
+ """Get asset groups."""
+ offset_value = offset.value if offset.value is not None else 0
+
+ group_query = (
+ select(AssetModel.group, func.count(AssetModel.id))
+ .where(
+ AssetModel.active.has() if only_active.value else True,
+ *([AssetModel.group == group] if group else []),
+ )
+ .group_by(AssetModel.group)
+ )
+ group_rows = session.execute(group_query).all()
+ limit_value = limit.value if limit.value is not None else len(group_rows)
+ # Ordene em Python
Review Comment:
Please english comments
##########
airflow-core/src/airflow/api_fastapi/core_api/routes/public/assets.py:
##########
@@ -206,6 +208,88 @@ def get_assets(
)
+@assets_router.get(
+ "/assets/groups",
+ responses=create_openapi_http_exception_doc([status.HTTP_404_NOT_FOUND]),
+ dependencies=[
+ Depends(requires_access_asset(method="GET")),
+ ],
+)
+def get_asset_groups(
+ limit: QueryLimit,
+ offset: QueryOffset,
+ only_active: Annotated[OnlyActiveFilter, Depends(OnlyActiveFilter.depends)],
+ session: SessionDep,
+ order_by: Annotated[str, Query(alias="order_by")] = "group",
+ group: str | None = Query(default=None, description="Filter by grupo"),
+) -> AssetGroupCollectionResponse:
+ """Get asset groups."""
+ offset_value = offset.value if offset.value is not None else 0
+
+ group_query = (
+ select(AssetModel.group, func.count(AssetModel.id))
+ .where(
+ AssetModel.active.has() if only_active.value else True,
+ *([AssetModel.group == group] if group else []),
+ )
+ .group_by(AssetModel.group)
+ )
+ group_rows = session.execute(group_query).all()
+ limit_value = limit.value if limit.value is not None else len(group_rows)
+ # Ordene em Python
+ if order_by == "count":
+ group_rows.sort(key=lambda x: (x[1], x[0]))
+ elif order_by == "-count":
+ group_rows.sort(key=lambda x: (-x[1], x[0]))
+ elif order_by == "-group":
+ group_rows.sort(key=lambda x: x[0], reverse=True)
+ else:
+ group_rows.sort(key=lambda x: x[0])
+ # Aplique paginação em Python
Review Comment:
same
##########
airflow-core/src/airflow/api_fastapi/core_api/routes/public/assets.py:
##########
@@ -206,6 +208,88 @@ def get_assets(
)
+@assets_router.get(
+ "/assets/groups",
+ responses=create_openapi_http_exception_doc([status.HTTP_404_NOT_FOUND]),
+ dependencies=[
+ Depends(requires_access_asset(method="GET")),
+ ],
+)
+def get_asset_groups(
+ limit: QueryLimit,
+ offset: QueryOffset,
+ only_active: Annotated[OnlyActiveFilter, Depends(OnlyActiveFilter.depends)],
+ session: SessionDep,
+ order_by: Annotated[str, Query(alias="order_by")] = "group",
+ group: str | None = Query(default=None, description="Filter by grupo"),
+) -> AssetGroupCollectionResponse:
+ """Get asset groups."""
+ offset_value = offset.value if offset.value is not None else 0
+
+ group_query = (
+ select(AssetModel.group, func.count(AssetModel.id))
+ .where(
+ AssetModel.active.has() if only_active.value else True,
+ *([AssetModel.group == group] if group else []),
+ )
+ .group_by(AssetModel.group)
+ )
+ group_rows = session.execute(group_query).all()
+ limit_value = limit.value if limit.value is not None else len(group_rows)
+ # Ordene em Python
+ if order_by == "count":
+ group_rows.sort(key=lambda x: (x[1], x[0]))
+ elif order_by == "-count":
+ group_rows.sort(key=lambda x: (-x[1], x[0]))
+ elif order_by == "-group":
+ group_rows.sort(key=lambda x: x[0], reverse=True)
+ else:
+ group_rows.sort(key=lambda x: x[0])
Review Comment:
What is that? Sorting should be on the db side, not on the application side.
##########
airflow-core/src/airflow/api_fastapi/common/parameters.py:
##########
@@ -665,7 +665,53 @@ def _transform_ti_states(states: list[str] | None) -> list[TaskInstanceState | N
),
]
+
# Assets
+class _AssetPatternFilterModel(BaseModel):
+ """Asset Pattern Filter Model with a match mode parameter."""
+
+ name_pattern: str | None = None
+ group_pattern: str | None = None
+ search_match_mode: Literal["any", "all"] | None = None
Review Comment:
Maybe don't do advanced match mode directly cause we would need to do some refactoring with `_TagsFilter` to have a BaseAdvance match mode filter to extend from. Because there's a lot duplicated.
##########
airflow-core/src/airflow/api_fastapi/core_api/services/ui/dependencies.py:
##########
@@ -59,20 +66,34 @@ def extract_single_connected_component(
connected_components = extract_connected_components(adjacency_matrix)
- filtered_connected_components = [cc for cc in connected_components if node_id in cc]
-
- if len(filtered_connected_components) != 1:
- raise ValueError(
- f"Unique connected component not found, got {filtered_connected_components} for connected components of node {node_id}, expected only 1 connected component."
- )
-
- connected_component = filtered_connected_components[0]
-
- nodes = [node for node in nodes if node["id"] in connected_component]
- edges = [
+ if len(node_ids) == 1:
+ node_id = node_ids[0]
+ filtered_connected_components = [cc for cc in connected_components if node_id in cc]
+ if len(filtered_connected_components) != 1:
+ raise ValueError(
+ f"Unique connected component not found, got {filtered_connected_components} for connected components of node {node_id}, expected only 1 connected component."
+ )
+ connected_component = filtered_connected_components[0]
+ filtered_nodes = [node for node in nodes if node["id"] in connected_component]
+ filtered_edges = [
+ edge
+ for edge in edges
+ if (edge["source_id"] in connected_component and edge["target_id"] in connected_component)
+ ]
+ return {"nodes": filtered_nodes, "edges": filtered_edges}
+ # Multiple node_ids: merge all relevant components
+ relevant_components = [cc for cc in connected_components if any(nid in cc for nid in node_ids)]
+ # NEW: Check that all node_ids are present in the merged set
+ if not relevant_components:
+ raise ValueError(f"No connected component found for node(s) {node_ids}.")
+ merged_node_ids = set().union(*relevant_components)
+ missing = [nid for nid in node_ids if nid not in merged_node_ids]
+ if missing:
+ raise ValueError(f"No connected component found for node(s) {missing}.")
+ filtered_nodes = [node for node in nodes if node["id"] in merged_node_ids]
+ filtered_edges = [
edge
for edge in edges
- if (edge["source_id"] in connected_component and edge["target_id"] in connected_component)
+ if (edge["source_id"] in merged_node_ids and edge["target_id"] in merged_node_ids)
Review Comment:
Needs refactoring, that's really hard to read .
##########
airflow-core/src/airflow/ui/src/pages/Asset/AssetGroupSidebar.tsx:
##########
@@ -0,0 +1,97 @@
+/*!
+ * 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, Text, HStack, VStack, Badge, Button } from "@chakra-ui/react";
+import { useTranslation } from "react-i18next";
+
+import type { AssetResponse } from "openapi/requests/types.gen";
+
+type AssetGroupSidebarProps = {
+ assets: Array<AssetResponse>;
+ groupName: string;
+};
+
+export const AssetGroupSidebar: React.FC<AssetGroupSidebarProps> = ({ assets, groupName }) => {
+ const { t: translate } = useTranslation(["assets", "common"]);
+
+ const producingTasks = assets.reduce((acc, asset) => acc + asset.producing_tasks.length, 0);
+ const consumingDags = assets.reduce((acc, asset) => acc + asset.consuming_tasks.length, 0);
+
+ // Collect all last asset events from all assets
+ const events = assets
+ .map((asset) => asset.last_asset_event)
+ .filter((ev): ev is NonNullable<typeof ev> => Boolean(ev));
+
+ return (
+ <VStack align="stretch" gap={4}>
+ <Box bg="chakra-subtle-bg" borderColor="chakra-border-color" borderRadius="md" borderWidth={1} p={4}>
+ <HStack gap={2} mb={2}>
+ <Badge aria-label={translate("group", { ns: "assets" })} colorScheme="gray" fontSize="lg">
+ {translate("databaseIcon", { ns: "assets" })}
+ </Badge>
+ <Heading size="md">{groupName}</Heading>
+ </HStack>
+ <Text color="chakra-fg" fontSize="sm" mb={2}>
+ <b>{translate("group", { ns: "assets" })}</b>
+ <br />
Review Comment:
use chakra.p, chakra.br, etc.
##########
airflow-core/src/airflow/api_fastapi/common/parameters.py:
##########
@@ -665,7 +665,53 @@ def _transform_ti_states(states: list[str] | None) -> list[TaskInstanceState | N
),
]
+
# Assets
+class _AssetPatternFilterModel(BaseModel):
+ """Asset Pattern Filter Model with a match mode parameter."""
+
+ name_pattern: str | None = None
+ group_pattern: str | None = None
+ search_match_mode: Literal["any", "all"] | None = None
Review Comment:
Those should probably be two different params, not just one.
And you can define them easily with the search_param_factory since they are not really doing more.
I don't think you need a custom class
```
QueryDagIdPatternSearch = Annotated[
_SearchParam, Depends(search_param_factory(DagModel.dag_id, "dag_id_pattern"))
]
QueryDagDisplayNamePatternSearch = Annotated[
_SearchParam, Depends(search_param_factory(DagModel.dag_display_name, "dag_display_name_pattern"))
]
```
##########
airflow-core/src/airflow/api_fastapi/core_api/routes/public/assets.py:
##########
@@ -206,6 +208,88 @@ def get_assets(
)
+@assets_router.get(
+ "/assets/groups",
+ responses=create_openapi_http_exception_doc([status.HTTP_404_NOT_FOUND]),
+ dependencies=[
+ Depends(requires_access_asset(method="GET")),
+ ],
+)
+def get_asset_groups(
+ limit: QueryLimit,
+ offset: QueryOffset,
+ only_active: Annotated[OnlyActiveFilter, Depends(OnlyActiveFilter.depends)],
+ session: SessionDep,
+ order_by: Annotated[str, Query(alias="order_by")] = "group",
+ group: str | None = Query(default=None, description="Filter by grupo"),
+) -> AssetGroupCollectionResponse:
+ """Get asset groups."""
+ offset_value = offset.value if offset.value is not None else 0
+
+ group_query = (
+ select(AssetModel.group, func.count(AssetModel.id))
+ .where(
+ AssetModel.active.has() if only_active.value else True,
+ *([AssetModel.group == group] if group else []),
+ )
+ .group_by(AssetModel.group)
+ )
Review Comment:
This will query all assets from the db grouped by asset group, but that could be thousands of them, that' not scalable.
--
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] feat(api/ui)#50779 : add asset group filtering and asset group views [airflow]
Posted by "pierrejeambrun (via GitHub)" <gi...@apache.org>.
pierrejeambrun commented on code in PR #51682:
URL: https://github.com/apache/airflow/pull/51682#discussion_r2161133729
##########
airflow-core/src/airflow/api_fastapi/core_api/routes/ui/dependencies.py:
##########
@@ -80,9 +95,26 @@ def get_dependencies(session: SessionDep, node_id: str | None = None) -> BaseGra
"edges": edges,
}
- if node_id is not None:
+ # If ids_to_fetch is filled, filter for each connected component
+ if ids_to_fetch:
try:
- data = extract_single_connected_component(node_id, data["nodes"], data["edges"])
+ # Join all connected components of the requested ids
+ all_nodes = []
+ all_edges = []
+ seen_nodes = set()
+ seen_edges = set()
+ for nid in ids_to_fetch:
+ comp = extract_single_connected_component(nid, data["nodes"], data["edges"])
+ for n in comp["nodes"]:
+ if n["id"] not in seen_nodes:
+ all_nodes.append(n)
+ seen_nodes.add(n["id"])
+ for e in comp["edges"]:
+ edge_tuple = (e["source_id"], e["target_id"])
+ if edge_tuple not in seen_edges:
+ all_edges.append(e)
+ seen_edges.add(edge_tuple)
+ data = {"nodes": all_nodes, "edges": all_edges}
Review Comment:
I don't think we should do this manually.
`extract_single_connected_component` is actually calling `extract_connected_components` which holds all the connected components of the graph. We could directly use all of those and merge them to avoid computing multiple times `extract_connected_components` (for each node id).
##########
airflow-core/src/airflow/ui/src/pages/AssetsGroupedList/AssetsGroupedList.tsx:
##########
@@ -0,0 +1,297 @@
+/*!
+ * 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, Link, IconButton, VStack, Flex, Heading, Text, Input } from "@chakra-ui/react";
+import { useState, useMemo } from "react";
+import { Link as RouterLink } from "react-router-dom";
+
+import { useAssetServiceGetAssets } from "openapi/queries";
+import type { AssetResponse } from "openapi/requests/types.gen";
+import { DataTable } from "src/components/DataTable";
+import { useTableURLState } from "src/components/DataTable/useTableUrlState";
+import { ErrorAlert } from "src/components/ErrorAlert";
+import Time from "src/components/Time";
+
+import { CreateAssetEvent } from "../Asset/CreateAssetEvent";
+import { DependencyPopover } from "../AssetsList/DependencyPopover";
+
+// Ícones SVG inline para expandir/colapsar
+const ChevronDownIcon = () => (
+ <svg fill="currentColor" height="1em" viewBox="0 0 20 20" width="1em">
+ <path
+ clipRule="evenodd"
+ d="M5.23 7.21a.75.75 0 011.06.02L10 11.085l3.71-3.855a.75.75 0 111.08 1.04l-4.24 4.4a.75.75 0 01-1.08 0l-4.24-4.4a.75.75 0 01.02-1.06z"
+ fillRule="evenodd"
+ />
+ </svg>
+);
+
+const ChevronRightIcon = () => (
+ <svg fill="currentColor" height="1em" viewBox="0 0 20 20" width="1em">
+ <path
+ clipRule="evenodd"
+ d="M7.21 5.23a.75.75 0 011.06-.02l4.4 4.24a.75.75 0 010 1.08l-4.4 4.24a.75.75 0 11-1.04-1.08L11.085 10 7.23 6.29a.75.75 0 01-.02-1.06z"
+ fillRule="evenodd"
+ />
+ </svg>
+);
Review Comment:
We already do this at different places in the code. (We use chevron icons from react-icon lib instead), you can do something similar.
##########
airflow-core/src/airflow/ui/src/queries/useMultiDependencyGraph.ts:
##########
@@ -0,0 +1,51 @@
+/*!
+ * 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 { useQueryClient, type UseQueryOptions } from "@tanstack/react-query";
+
+import {
+ useDependenciesServiceGetDependencies,
+ UseDependenciesServiceGetDependenciesKeyFn,
+} from "openapi/queries";
+import type { BaseGraphResponse } from "openapi/requests/types.gen";
+
+/**
+ * Hook to fetch the dependency graph for multiple assets in a single query.
+ * Expects the backend to accept nodeIds as a comma-separated string or array.
+ */
+export const useMultiDependencyGraph = (
+ assetIds: Array<string>,
+ options?: Omit<UseQueryOptions<BaseGraphResponse, unknown>, "queryFn" | "queryKey">,
+) => {
+ const queryClient = useQueryClient();
+
+ const nodeIdsParam = assetIds.map((id) => `asset:${id}`).join(",");
+
+ const query = useDependenciesServiceGetDependencies({ nodeIds: nodeIdsParam }, undefined, options);
+
+ query.data?.nodes.forEach((node) => {
+ const key = UseDependenciesServiceGetDependenciesKeyFn({ nodeId: node.id });
+ const queryData = queryClient.getQueryData(key);
+
+ if (!Boolean(queryData)) {
+ queryClient.setQueryData(key, query.data);
+ }
+ });
Review Comment:
Nice idea but I think in its current form it could induce some bugs. Setting the cache for a particular node_id, with the result of `useDependenciesServiceGetDependencies` with many node ids. Because the response returned for many node ids will hold more connected components than just the 1 single connected component that we should retrieve from the `nodeId: node.id` filter.
This could therefore display unrelated dependencies when displaying dependencies of a particular node.
##########
airflow-core/src/airflow/ui/src/pages/AssetsList/AssetsList.tsx:
##########
@@ -92,64 +105,111 @@ const createColumns = (translate: (key: string) => string): Array<ColumnDef<Asse
},
];
-const NAME_PATTERN_PARAM = SearchParamsKeys.NAME_PATTERN;
-
export const AssetsList = () => {
const { t: translate } = useTranslation(["assets", "common"]);
const [searchParams, setSearchParams] = useSearchParams();
- const [namePattern, setNamePattern] = useState(searchParams.get(NAME_PATTERN_PARAM) ?? undefined);
+ const [searchValue, setSearchValue] = useState(
+ searchParams.get(SearchParamsKeys.NAME_PATTERN) ??
+ searchParams.get(SearchParamsKeys.GROUP_PATTERN) ??
+ undefined,
+ );
const { setTableURLState, tableURLState } = useTableURLState();
const { pagination, sorting } = tableURLState;
const [sort] = sorting;
const orderBy = sort ? `${sort.desc ? "-" : ""}${sort.id}` : undefined;
- const { data, error, isLoading } = useAssetServiceGetAssets({
+ const {
+ data: dataByName,
+ error: errorByName,
+ isLoading: isLoadingByName,
+ } = useAssetServiceGetAssets({
limit: pagination.pageSize,
- namePattern,
+ namePattern: searchValue ?? undefined,
offset: pagination.pageIndex * pagination.pageSize,
orderBy,
});
+ const {
+ data: dataByGroup,
+ error: errorByGroup,
+ isLoading: isLoadingByGroup,
+ } = useAssetServiceGetAssets({
+ groupPattern: searchValue ?? undefined,
+ limit: pagination.pageSize,
+ offset: pagination.pageIndex * pagination.pageSize,
+ orderBy,
+ });
Review Comment:
We shouldn't do that manually in the UI. (call the endpoints with two different parameters and then merge results in the UI).
The backend should support this. (You can write a custom filter that will do the search accross both 'asset.name` and `asset.group`, or expand the `search_param_factory` and `_SearchParam` to be able to take a `list[ColumnElement]` as attribute. And perform a search across multiple columns.
##########
airflow-core/src/airflow/ui/src/pages/AssetsGroupedList/AssetsGroupedList.tsx:
##########
@@ -0,0 +1,297 @@
+/*!
+ * 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, Link, IconButton, VStack, Flex, Heading, Text, Input } from "@chakra-ui/react";
+import { useState, useMemo } from "react";
+import { Link as RouterLink } from "react-router-dom";
+
+import { useAssetServiceGetAssets } from "openapi/queries";
+import type { AssetResponse } from "openapi/requests/types.gen";
+import { DataTable } from "src/components/DataTable";
+import { useTableURLState } from "src/components/DataTable/useTableUrlState";
+import { ErrorAlert } from "src/components/ErrorAlert";
+import Time from "src/components/Time";
+
+import { CreateAssetEvent } from "../Asset/CreateAssetEvent";
+import { DependencyPopover } from "../AssetsList/DependencyPopover";
+
+// Ícones SVG inline para expandir/colapsar
+const ChevronDownIcon = () => (
+ <svg fill="currentColor" height="1em" viewBox="0 0 20 20" width="1em">
+ <path
+ clipRule="evenodd"
+ d="M5.23 7.21a.75.75 0 011.06.02L10 11.085l3.71-3.855a.75.75 0 111.08 1.04l-4.24 4.4a.75.75 0 01-1.08 0l-4.24-4.4a.75.75 0 01.02-1.06z"
+ fillRule="evenodd"
+ />
+ </svg>
+);
+
+const ChevronRightIcon = () => (
+ <svg fill="currentColor" height="1em" viewBox="0 0 20 20" width="1em">
+ <path
+ clipRule="evenodd"
+ d="M7.21 5.23a.75.75 0 011.06-.02l4.4 4.24a.75.75 0 010 1.08l-4.4 4.24a.75.75 0 11-1.04-1.08L11.085 10 7.23 6.29a.75.75 0 01-.02-1.06z"
+ fillRule="evenodd"
+ />
+ </svg>
+);
+
+const NameCell = ({ original }: { readonly original: AssetResponse }) => (
+ <Link asChild color="fg.info" fontWeight="bold">
+ <RouterLink to={`/assets/${original.id}`}>{original.name}</RouterLink>
+ </Link>
+);
+
+const LastAssetEventCell = ({ original }: { readonly original: AssetResponse }) => {
+ const assetEvent = original.last_asset_event;
+ const timestamp = assetEvent?.timestamp;
+
+ if (timestamp === undefined || timestamp === null) {
+ return undefined;
+ }
+
+ return <Time datetime={timestamp} />;
+};
+
+const GroupCell = ({ original }: { readonly original: AssetResponse }) => {
+ const { group } = original;
+
+ if (!group) {
+ return undefined;
+ }
+
+ return (
+ <Link asChild color="fg.info" fontWeight="bold">
+ <RouterLink to={`/assets/group/${group}`}>{group}</RouterLink>
+ </Link>
+ );
+};
+
+const ConsumingDagsCell = ({ original }: { readonly original: AssetResponse }) =>
+ original.consuming_dags.length ? (
+ <DependencyPopover dependencies={original.consuming_dags} type="Dag" />
+ ) : undefined;
+
+const ProducingTasksCell = ({ original }: { readonly original: AssetResponse }) =>
+ original.producing_tasks.length ? (
+ <DependencyPopover dependencies={original.producing_tasks} type="Task" />
+ ) : undefined;
+
+const TriggerCell = ({ original }: { readonly original: AssetResponse }) => (
+ <CreateAssetEvent asset={original} withText={false} />
+);
+
+const truncate = (str: string, max = 32) => (str.length > max ? `${str.slice(0, max - 3)}...` : str);
+
+const nameCellRenderer = ({ row: { original } }: { row: { original: AssetResponse } }) => (
+ <NameCell original={original} />
+);
+const lastAssetEventCellRenderer = ({ row: { original } }: { row: { original: AssetResponse } }) => (
+ <LastAssetEventCell original={original} />
+);
+const groupCellRenderer = ({ row: { original } }: { row: { original: AssetResponse } }) => (
+ <GroupCell original={original} />
+);
+const consumingDagsCellRenderer = ({ row: { original } }: { row: { original: AssetResponse } }) => (
+ <ConsumingDagsCell original={original} />
+);
+const producingTasksCellRenderer = ({ row: { original } }: { row: { original: AssetResponse } }) => (
+ <ProducingTasksCell original={original} />
+);
+const triggerCellRenderer = ({ row: { original } }: { row: { original: AssetResponse } }) => (
+ <TriggerCell original={original} />
+);
+
+export const AssetsGroupedList = () => {
+ const { setTableURLState, tableURLState } = useTableURLState();
+ const { sorting } = tableURLState;
+ const [sort] = sorting;
+ const orderBy = sort ? `${sort.desc ? "-" : ""}${sort.id}` : undefined;
+
+ const { data, error, isLoading } = useAssetServiceGetAssets({
+ limit: 1000,
+ offset: 0,
+ orderBy,
+ });
+
+ const [search, setSearch] = useState("");
+ const [sortAsc, setSortAsc] = useState(true);
+
+ // Group assets by group
+ const grouped = useMemo(() => {
+ const filtered = (data?.assets ?? []).filter((asset) =>
+ asset.group.toLowerCase().includes(search.toLowerCase()),
+ );
+ const groupedObj = filtered.reduce<Record<string, Array<AssetResponse>>>((acc, asset) => {
+ const groupName = asset.group;
+
+ acc[groupName] ??= [];
+ acc[groupName].push(asset);
+
+ return acc;
+ }, {});
Review Comment:
I think this logic can holds a few problems, first of the endpoint `useAssetServiceGetAssets` is paginated. It means that you'll only retrieve the first 50 items, and you need a mechanism to display the remaining items (cf tables pagination component).
In addition, there is no guarantee at this point that all assets for a specified group will be on the same page, you could end up with a group missing some assets because those were not returned yet.
I think the easiest approach would be to add a backend ui endpoints to list/retrieve asset groups directly. (I group would list all of it's child asset and we would paginate on groups directly). Maybe there are other approaches possible but I would need to think about it.
##########
airflow-core/src/airflow/api_fastapi/core_api/routes/ui/dependencies.py:
##########
@@ -41,12 +40,28 @@
),
dependencies=[Depends(requires_access_dag("GET", DagAccessEntity.DEPENDENCIES))],
)
-def get_dependencies(session: SessionDep, node_id: str | None = None) -> BaseGraphResponse:
- """Dependencies graph."""
+def get_dependencies(
+ node_id: str | None = None,
+ node_ids: str | None = Query(None, description="Comma-separated list of node ids"),
+) -> BaseGraphResponse:
+ """Dependencies graph. Supports a single node_id or multiple node_ids separated by commas."""
+ # Parse node_ids (priority to node_ids, fallback to node_id)
+ ids_to_fetch: list[str] = []
+ # If node_id contains commas, treat as multiple IDs (extra protection)
+ if node_ids:
+ ids_to_fetch = [nid.strip() for nid in node_ids.split(",") if nid.strip()]
Review Comment:
No, FastAPI handles `node_ids: list[str] | None = None` natively. FastAPI chose exploded form query params, i.e `?node_ids=1&node_ids=2` you will directly receive `node_ids=["1","2"] in your function.
--
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] feat(api/ui)#50779 : add asset group filtering and asset group views [airflow]
Posted by "davidfgcorreia (via GitHub)" <gi...@apache.org>.
davidfgcorreia commented on PR #51682:
URL: https://github.com/apache/airflow/pull/51682#issuecomment-2971315267
Runned precommit locally and got zero errors how can I replicate this setup and make sure it passes consistently?
--
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] feat(api/ui)#50779 : add asset group filtering and asset group views [airflow]
Posted by "guan404ming (via GitHub)" <gi...@apache.org>.
guan404ming commented on code in PR #51682:
URL: https://github.com/apache/airflow/pull/51682#discussion_r2157645530
##########
airflow-core/src/airflow/ui/src/pages/Asset/AssetGroupSidebar.tsx:
##########
@@ -0,0 +1,94 @@
+/*!
+ * 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, Text, HStack, VStack, Badge, Button } from "@chakra-ui/react";
+
+import type { AssetResponse } from "openapi/requests/types.gen";
+
+type AssetGroupSidebarProps = {
+ assets: Array<AssetResponse>;
+ groupName: string;
+};
+
+export const AssetGroupSidebar: React.FC<AssetGroupSidebarProps> = ({ assets, groupName }) => {
+ const producingTasks = assets.reduce((acc, asset) => acc + asset.producing_tasks.length, 0);
+ const consumingDags = assets.reduce((acc, asset) => acc + asset.consuming_dags.length, 0);
+
+ // Collect all last asset events from all assets
+ const events = assets
+ .map((asset) => asset.last_asset_event)
+ .filter((ev): ev is NonNullable<typeof ev> => Boolean(ev));
+
+ return (
+ <VStack align="stretch" gap={4}>
+ <Box bg="chakra-subtle-bg" borderColor="chakra-border-color" borderRadius="md" borderWidth={1} p={4}>
+ <HStack gap={2} mb={2}>
+ <Badge aria-label="database icon" colorScheme="gray" fontSize="lg">
+ 🗄️
+ </Badge>
+ <Heading size="md">{groupName}</Heading>
+ </HStack>
+ <Text color="chakra-fg" fontSize="sm" mb={2}>
+ <b>{/* i18n: Group label */}Group</b>
+ <br />
+ {groupName}
+ </Text>
+ <HStack gap={4} mb={2}>
+ <Box>
+ <Text color="chakra-fg" fontSize="xs">
+ {/* i18n: Producing Tasks label */}Producing Tasks
Review Comment:
We currently disable literal string in ui due to i18n thus you need to update the translation file `en/assets.json` or `en/common.json` and use `useTranslation` hook to access them. You could also run `npm run eslint` in ui folder to see your lint error.
##########
airflow-core/src/airflow/ui/src/pages/Asset/AssetGroupSidebar.tsx:
##########
@@ -0,0 +1,94 @@
+/*!
+ * 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, Text, HStack, VStack, Badge, Button } from "@chakra-ui/react";
+
+import type { AssetResponse } from "openapi/requests/types.gen";
+
+type AssetGroupSidebarProps = {
+ assets: Array<AssetResponse>;
+ groupName: string;
+};
+
+export const AssetGroupSidebar: React.FC<AssetGroupSidebarProps> = ({ assets, groupName }) => {
+ const producingTasks = assets.reduce((acc, asset) => acc + asset.producing_tasks.length, 0);
+ const consumingDags = assets.reduce((acc, asset) => acc + asset.consuming_dags.length, 0);
+
+ // Collect all last asset events from all assets
+ const events = assets
+ .map((asset) => asset.last_asset_event)
+ .filter((ev): ev is NonNullable<typeof ev> => Boolean(ev));
+
+ return (
+ <VStack align="stretch" gap={4}>
+ <Box bg="chakra-subtle-bg" borderColor="chakra-border-color" borderRadius="md" borderWidth={1} p={4}>
+ <HStack gap={2} mb={2}>
+ <Badge aria-label="database icon" colorScheme="gray" fontSize="lg">
+ 🗄️
+ </Badge>
+ <Heading size="md">{groupName}</Heading>
+ </HStack>
+ <Text color="chakra-fg" fontSize="sm" mb={2}>
+ <b>{/* i18n: Group label */}Group</b>
+ <br />
+ {groupName}
+ </Text>
+ <HStack gap={4} mb={2}>
+ <Box>
+ <Text color="chakra-fg" fontSize="xs">
+ {/* i18n: Producing Tasks label */}Producing Tasks
Review Comment:
We currently disable literal string in ui due to i18n thus you need to update the translation file like `en/assets.json` or `en/common.json` and use `useTranslation` hook to access them. You could also run `npm run eslint` in ui folder to see your lint error.
--
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] feat(api/ui)#50779 : add asset group filtering and asset group views [airflow]
Posted by "davidfgcorreia (via GitHub)" <gi...@apache.org>.
davidfgcorreia commented on PR #51682:
URL: https://github.com/apache/airflow/pull/51682#issuecomment-3016817302
Hi, the pipeline is currently failing on Tests AMD / CI image checks / Static checks (pull_request), specifically at the step "Generate SVG from Airflow CTL Commands".
However, when I run the checks locally:
pre-commit run (on modified files only) skips this check, as expected, since I haven't modified any related files;
pre-commit run --all-files passes the check successfully, including the "Generate SVG from Airflow CTL Commands" step.
Please let me know if there's anything else I should look into or adjust. Thanks!
--
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] feat(api/ui)#50779 : add asset group filtering and asset group views [airflow]
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on PR #51682:
URL: https://github.com/apache/airflow/pull/51682#issuecomment-3016824344
> Hi, the pipeline is currently failing on Tests AMD / CI image checks / Static checks (pull_request), specifically at the step "Generate SVG from Airflow CTL Commands".
>
> However, when I run the checks locally:
>
> pre-commit run (on modified files only) skips this check, as expected, since I haven't modified any related files;
>
> pre-commit run --all-files passes the check successfully, including the "Generate SVG from Airflow CTL Commands" step.
>
> Please let me know if there's anything else I should look into or adjust. Thanks!
That was temportary. As usual. Rebase. Always Be Rebased. There is another issue in main being fixed as we speak though so you might wait a few minutes.
--
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] feat(api/ui)#50779 : add asset group filtering and asset group views [airflow]
Posted by "pierrejeambrun (via GitHub)" <gi...@apache.org>.
pierrejeambrun commented on PR #51682:
URL: https://github.com/apache/airflow/pull/51682#issuecomment-3057093498
@davidfgcorreia that could be transient, unrelated, you can close and reopen the PR to retrigger the CI, or just rebase or just push an amended commit.
--
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] feat(api/ui)#50779 : add asset group filtering and asset group views [airflow]
Posted by "davidfgcorreia (via GitHub)" <gi...@apache.org>.
davidfgcorreia commented on PR #51682:
URL: https://github.com/apache/airflow/pull/51682#issuecomment-2993473868
> Thanks, this feature is great. Feel free to ask anything.
Yes, I have a question. I created a new file called sidebar.tsx, but I'm not able to catch the errors in it , even though the runner here on Git is marking them. Do you know what might be going on?
--
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] feat(api/ui)#50779 : add asset group filtering and asset group views [airflow]
Posted by "guan404ming (via GitHub)" <gi...@apache.org>.
guan404ming commented on code in PR #51682:
URL: https://github.com/apache/airflow/pull/51682#discussion_r2157645530
##########
airflow-core/src/airflow/ui/src/pages/Asset/AssetGroupSidebar.tsx:
##########
@@ -0,0 +1,94 @@
+/*!
+ * 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, Text, HStack, VStack, Badge, Button } from "@chakra-ui/react";
+
+import type { AssetResponse } from "openapi/requests/types.gen";
+
+type AssetGroupSidebarProps = {
+ assets: Array<AssetResponse>;
+ groupName: string;
+};
+
+export const AssetGroupSidebar: React.FC<AssetGroupSidebarProps> = ({ assets, groupName }) => {
+ const producingTasks = assets.reduce((acc, asset) => acc + asset.producing_tasks.length, 0);
+ const consumingDags = assets.reduce((acc, asset) => acc + asset.consuming_dags.length, 0);
+
+ // Collect all last asset events from all assets
+ const events = assets
+ .map((asset) => asset.last_asset_event)
+ .filter((ev): ev is NonNullable<typeof ev> => Boolean(ev));
+
+ return (
+ <VStack align="stretch" gap={4}>
+ <Box bg="chakra-subtle-bg" borderColor="chakra-border-color" borderRadius="md" borderWidth={1} p={4}>
+ <HStack gap={2} mb={2}>
+ <Badge aria-label="database icon" colorScheme="gray" fontSize="lg">
+ 🗄️
+ </Badge>
+ <Heading size="md">{groupName}</Heading>
+ </HStack>
+ <Text color="chakra-fg" fontSize="sm" mb={2}>
+ <b>{/* i18n: Group label */}Group</b>
+ <br />
+ {groupName}
+ </Text>
+ <HStack gap={4} mb={2}>
+ <Box>
+ <Text color="chakra-fg" fontSize="xs">
+ {/* i18n: Producing Tasks label */}Producing Tasks
Review Comment:
We currently disable literal string due to i18n thus you need to update the translation file `en/assets.json` or `en/common.json` and use `useTranslation` hook to access them. You could also run `npm run eslint` in ui folder to see your lint error.
--
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] feat(api/ui)#50779 : add asset group filtering and asset group views [airflow]
Posted by "davidfgcorreia (via GitHub)" <gi...@apache.org>.
davidfgcorreia commented on PR #51682:
URL: https://github.com/apache/airflow/pull/51682#issuecomment-3172854955
I can't work on this issue no longer
--
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] feat(api/ui)#50779 : add asset group filtering and asset group views [airflow]
Posted by "pierrejeambrun (via GitHub)" <gi...@apache.org>.
pierrejeambrun commented on PR #51682:
URL: https://github.com/apache/airflow/pull/51682#issuecomment-2995971556
> Yes, I have a question. I created a new file called sidebar.tsx, but I'm not able to catch the errors in it , even though the runner here on Git is marking them. Do you know what might be going on?
Be sure to be rebased on latest main with latest devtools updates (We recently merged a few improvement to this part). You can then run pre-commit hooks or eslint:
- `pre-commit run ts-compile-lint-ui` (with your files stages) or `pre-commit run ts-compile-lint-ui --all-files` (for all files)
- or `pnpm eslint --fix your_file`
--
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] feat(api/ui)#50779 : add asset group filtering and asset group views [airflow]
Posted by "davidfgcorreia (via GitHub)" <gi...@apache.org>.
davidfgcorreia closed pull request #51682: feat(api/ui)#50779 : add asset group filtering and asset group views
URL: https://github.com/apache/airflow/pull/51682
--
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] feat(api/ui)#50779 : add asset group filtering and asset group views [airflow]
Posted by "jason810496 (via GitHub)" <gi...@apache.org>.
jason810496 commented on PR #51682:
URL: https://github.com/apache/airflow/pull/51682#issuecomment-3171958514
Hi @davidfgcorreia, thanks for the contribution!
May I ask if this PR was closed by accident?
--
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] feat(api/ui)#50779 : add asset group filtering and asset group views [airflow]
Posted by "guan404ming (via GitHub)" <gi...@apache.org>.
guan404ming commented on code in PR #51682:
URL: https://github.com/apache/airflow/pull/51682#discussion_r2157645530
##########
airflow-core/src/airflow/ui/src/pages/Asset/AssetGroupSidebar.tsx:
##########
@@ -0,0 +1,94 @@
+/*!
+ * 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, Text, HStack, VStack, Badge, Button } from "@chakra-ui/react";
+
+import type { AssetResponse } from "openapi/requests/types.gen";
+
+type AssetGroupSidebarProps = {
+ assets: Array<AssetResponse>;
+ groupName: string;
+};
+
+export const AssetGroupSidebar: React.FC<AssetGroupSidebarProps> = ({ assets, groupName }) => {
+ const producingTasks = assets.reduce((acc, asset) => acc + asset.producing_tasks.length, 0);
+ const consumingDags = assets.reduce((acc, asset) => acc + asset.consuming_dags.length, 0);
+
+ // Collect all last asset events from all assets
+ const events = assets
+ .map((asset) => asset.last_asset_event)
+ .filter((ev): ev is NonNullable<typeof ev> => Boolean(ev));
+
+ return (
+ <VStack align="stretch" gap={4}>
+ <Box bg="chakra-subtle-bg" borderColor="chakra-border-color" borderRadius="md" borderWidth={1} p={4}>
+ <HStack gap={2} mb={2}>
+ <Badge aria-label="database icon" colorScheme="gray" fontSize="lg">
+ 🗄️
+ </Badge>
+ <Heading size="md">{groupName}</Heading>
+ </HStack>
+ <Text color="chakra-fg" fontSize="sm" mb={2}>
+ <b>{/* i18n: Group label */}Group</b>
+ <br />
+ {groupName}
+ </Text>
+ <HStack gap={4} mb={2}>
+ <Box>
+ <Text color="chakra-fg" fontSize="xs">
+ {/* i18n: Producing Tasks label */}Producing Tasks
Review Comment:
We currently disable literal string in ui due to i18n thus you need to update the translation file like `en/assets.json` or `en/common.json` and use `useTranslation` hook to access them. You could also run `npm run lint` in ui folder to see your lint error.
--
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] feat(api/ui)#50779 : add asset group filtering and asset group views [airflow]
Posted by "davidfgcorreia (via GitHub)" <gi...@apache.org>.
davidfgcorreia commented on PR #51682:
URL: https://github.com/apache/airflow/pull/51682#issuecomment-3041372232
After my last changes, the scheduler query count tests are failing with a much higher number of queries than expected, even though I did not touch any scheduler or ORM/database logic. I double-checked and only changed API and UI code. Has anyone seen this before or have any idea what could cause this?
--
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] feat(api/ui)#50779 : add asset group filtering and asset group views [airflow]
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on PR #51682:
URL: https://github.com/apache/airflow/pull/51682#issuecomment-3016825013
This is a PR that targets fixing the current main "generate constraints" failure https://github.com/apache/airflow/pull/52464 - once merged, rebasing your PR should fix unrelated 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] feat(api/ui)#50779 : add asset group filtering and asset group views [airflow]
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on PR #51682:
URL: https://github.com/apache/airflow/pull/51682#issuecomment-3016827245
Rebased it for you after merging my 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