fix: Continue and break to fill in conversation details

[shaohuzhang1]

fix: Continue and break to fill in conversation details

[f2c-ci-robot]

Adding the "do-not-merge/release-note-label-needed" label because no release-note block was detected, please follow our release note process to remove it.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

[f2c-ci-robot]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:

The full list of commands accepted by this bot can be found here.

Details

Needs approval from an approver in each of these files:

• OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[shaohuzhang1]

The provided code contains the following adjustments to improve its clarity and functionality:

1. Removed unnecessary import statement: from typing import Union.
2. Corrected a logic error in setting the value of _write_context based on __name__.endswith("execute"). This should be corrected to match the original intention of writing context when executing nodes.
3. Changed the order of operations in calculating self.node_params['is_result'] to align with the expected behavior.

After these changes, the modified line for determining whether to set self.node_params['is_result'] would look like this:

is_success = (not all([r if t == 'OR' else not r]) if condition == 'AND' else ((not any([(True if t != 'OR' else False) if r else False] for r, t in zip(condition_list, condition.split(','))))))
self.node_params['is_result'] = is_success

This adjustment ensures that the logical operation correctly matches the intended flow specified by condition, especially important given the potential for different combinations ("OR" vs "AND") within the condition_list.

These improvements will make it easier to understand the purpose and functionality each part of the code represents.

[shaohuzhang1]

There are no significant errors in the provided code. However, there's one minor issue with handling None values that might occur if condition_list contains NaN. It would be good to handle such cases appropriately.

Here’s an updated version of the code:

def execute(self, condition, condition_list, **kwargs) -> NodeResult:
    # Ensure condition_list is not None and convert it to a list if needed
    condition_list = condition_list or []
    
    # Create a new list that handles NaN (if present) more elegantly
    condition_list_with_nan_fix = [row.get('field') and self.assertion(
        row.get('field'), 
        row.get('compare'), 
        row.get('value', {}).get('original_value')
    ) for row in condition_list]

    # Use boolean logic based on the input 'condition'
    is_continue = all(condition_list_with_nan_fix) if condition == 'and' else any(condition_list_with_nan_fix)

    # Store the result and update context accordingly
    self.context['is_continue'] = is_continue

    # Return the appropriate outcome based on 'is_continue'
    if is_continue:
        return NodeResult({'is_continue': is_continue, 'branch_id': 'continue'}, {})
    return NodeResult({'is_continue': is_continue}, {})

def get_details(self, index: int, **kwargs):
    return {
        'name': self.node.properties.get('stepName'),
        "index": index,
        'is_continue': self.context.get('is_continue', False),
        'run_time': self.context.get('run_time'),
        'type': self.node.type,
        'status': self.status
    }

Changes Made:

1. Handling Non-List Condition List: Added a check at the beginning of execute() to ensure condition_list is a list and initialize it as empty if it is None.

   condition_list = condition_list or []

2. Preserving Nan Values: Updated each assertion call to include optional access to 'original_value', which may help in treating NaN gracefully.

   row.get('value', {}).get('original_value')

3. Default Value for Context Key: Added a default value False for "is_continue" in the get_details() function to avoid potential errors when accessing a missing key.

   'is_continue': self.context.get('is_continue', False)

These changes make the code more robust against unexpected inputs and improve the overall reliability of the node execution logic.