bbovenzi commented on code in PR #27987:
URL: https://github.com/apache/airflow/pull/27987#discussion_r1036463699
##########
airflow/www/static/js/api/useDatasetDependencies.ts:
##########
@@ -82,111 +85,89 @@ const generateGraph = ({ nodes, edges, font }:
GenerateProps) => ({
edges: edges.map((e) => ({ id: `${e.source}-${e.target}`, sources:
[e.source], targets: [e.target] })),
});
-interface SeparateGraphsProps extends DatasetDependencies {
- graphs: DatasetDependencies[];
+interface SeparateGraphsProps {
+ edges: DepEdge[];
+ graphs: EdgeGroup[];
}
-const graphIndicesToMerge: Record<number, number[]> = {};
-const indicesToRemove: number[] = [];
-
// find the downstream graph of each upstream edge
const findDownstreamGraph = (
- { edges, nodes, graphs = [] }: SeparateGraphsProps,
-): DatasetDependencies[] => {
- const newGraphs = [...graphs];
+ { edges, graphs = [] }: SeparateGraphsProps,
+): EdgeGroup[] => {
let filteredEdges = [...edges];
- graphs.forEach((g, i) => {
- // find downstream edges
- const downstreamEdges = edges.filter((e) => g.edges.some((ge) => ge.target
=== e.source));
- const downstreamNodes: DepNode[] = [];
-
- downstreamEdges.forEach((e) => {
- const newNode = nodes.find((n) => n.id === e.target);
- if (newNode) {
- downstreamNodes.push(newNode);
-
- // check if the node already exists in a different graph
- const existingGraphIndex = newGraphs
- .findIndex(((ng) => ng.nodes.some((n) => n.id === newNode.id)));
-
- // mark if the graph needs to merge with another
- if (existingGraphIndex > -1) {
- indicesToRemove.push(existingGraphIndex);
- graphIndicesToMerge[i] = [...(graphIndicesToMerge[i] || []),
existingGraphIndex];
- }
-
- // add node and edge to the graph
- newGraphs[i] = {
- nodes: [...newGraphs[i].nodes, newNode],
- edges: [...newGraphs[i].edges, e],
- };
-
- // remove edge from edge list
- filteredEdges = filteredEdges
- .filter((fe) => !(fe.source === e.source && fe.target === e.target));
+ // merge graphs that share edges
+ const mergedGraphs = graphs.reduce(
+ (newGraphs, g, i) => {
+ const otherGroupIndex = newGraphs.findIndex(
+ (og) => og.edges.some(
+ (oge) => g.edges.some(
+ (e) => e.target === oge.target,
+ ),
+ ),
+ );
+ if (otherGroupIndex === -1) {
+ return [...newGraphs, g];
}
+ const mergedEdges = [...newGraphs[otherGroupIndex].edges, ...g.edges]
+ .filter((e, j, totalEdges) => (
+ j === totalEdges.findIndex((t) => t.source === e.source && t.target
=== e.target)
+ ));
+ return [
+ ...newGraphs.filter((_, k) => k !== i && k !== otherGroupIndex),
Review Comment:
You're right. That was left over from an older fix to the problem.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
