Skip to content
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 12 additions & 4 deletions apps/application/flow/workflow_manage.py
Original file line number Diff line number Diff line change
Expand Up @@ -680,10 +680,16 @@ def get_next_node(self):
return None

@staticmethod
def dependent_node(up_node_id, node):
def dependent_node(edge, node):
up_node_id = edge.sourceNodeId
if not node.node_chunk.is_end():
return False
if node.id == up_node_id:
if node.context.get('branch_id', None):
if edge.sourceAnchorId == f"{node.id}_{node.context.get('branch_id', None)}_right":
return True
else:
return False
if node.type == 'form-node':
if node.context.get('form_data', None) is not None:
return True
Expand All @@ -696,9 +702,11 @@ def dependent_node_been_executed(self, node_id):
@param node_id: 需要判断的节点id
@return:
"""
up_node_id_list = [edge.sourceNodeId for edge in self.flow.edges if edge.targetNodeId == node_id]
return all([any([self.dependent_node(up_node_id, node) for node in self.node_context]) for up_node_id in
up_node_id_list])
up_edge_list = [edge for edge in self.flow.edges if edge.targetNodeId == node_id]
return all(
[any([self.dependent_node(edge, node) for node in self.node_context if node.id == edge.sourceNodeId]) for
edge in
up_edge_list])

def get_up_node_id_list(self, node_id):
up_node_id_list = [edge.sourceNodeId for edge in self.flow.edges if edge.targetNodeId == node_id]
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The provided code seems to be part of a method that checks whether a given node has any dependencies from its upstream nodes, particularly those within an ongoing flow or process. There are a few potential issues and improvements:

  1. Use of Static Method: The dependent_node method is currently implemented as static but accesses non-static attributes (edge.sourceNodeId). This might lead to unexpected behavior if used incorrectly.

  2. Variable Naming Convention: While consistent with Python variable naming conventions (using lowercase letters separated by underscores), there could still be minor inconsistencies in variable names like up_edge_list.

  3. Redundant List Comprehension: In the get_next_node method, you have multiple lines similar to each other but perform almost identical operations on lists. Reducing redundancy can improve readability and efficiency.

  4. Edge Validation: Ensure that every edge source or target node exists in the self.node_context dictionary. Missing or extraneous edges might cause errors later in the function or during execution.

  5. Context Handling: The context handling for different types of nodes (e.g., 'form-node') assumes specific key values ('branch_id' and 'form_data'). Consider adding error checks or logging when these keys aren't present to handle exceptions gracefully.

Here's a revised version with some suggested optimizations and corrections:

def get_next_node(self):
    # Check if we need to fetch data here or elsewhere
    pass

@staticmethod
def dependent_node(edge, node):
    # Validate input parameters
    if not isinstance(edge, dict) or "sourceNodeId" not in edge:
        raise ValueError("Invalid edge input")
    
    up_node_id = edge["sourceNodeId"]
    node_context = {node.id: node}
    
    if not node.node_chunk.is_end():
        return False
    
    if node.id == up_node_id:
        branch_info = node.context.get('branch_id')
        if branch_info:
            right_anchor = f"{node.id}_{branch_info}_right"
            return edge.sourceAnchorId == right_anchor
        elif node.type == 'form-node':
            form_data = node.context.get('form_data')
            return form_data is not None
        
    return False

def dependent_node_been_executed(self, node_id):
    up_edges = list(filter(lambda e: e.targetNodeId == node_id, self.flow.edges))
    is_dependent = lambda n_ctx, edg: any(self.dependent_node(edg, n_ctx))
    return all(is_dependent(node_ctx, edg) for node_ctx in self.node_context.values() for edg in up_edges)

def get_up_node_id_list(self, node_id):
    return [edg.sourceNodeId for edg in self.flow.edges if edg.targetNodeId == node_id]

In this revision:

  • I've added type checking in dependent_node to ensure it receives a valid edge object.
  • Replaced complex conditional logic with more concise checks using lambda.
  • Removed unnecessary variables like is_dependent and filtered_up_edges, keeping the logic straightforward.

This should address most of the issues mentioned while maintaining functionality.

Expand Down
Loading