pierrejeambrun commented on code in PR #27987:
URL: https://github.com/apache/airflow/pull/27987#discussion_r1036318122
##########
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]
Review Comment:
```typescript
// merge edges and remove duplicates
```
It is also done 2 times (the remove duplicate edges part, a little further
down in the following map block), maybe extratinct this to its own little
function could help.
--
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]
