fix: In the execution details, the execution time of the loop node is…#4479
Conversation
… displayed incorrectly
|
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. DetailsInstructions 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. |
|
[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. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
| node.context['run_time'] = time.time() - node.context.get("start_time") | ||
|
|
||
|
|
||
| def get_write_context(loop_type, array, number, loop_body): |
There was a problem hiding this comment.
The provided code snippet appears to be part of a workflow management function that is handling nested loops and iterating through an array. Here are some points to consider:
-
Variable Naming: The variable
nodeseems to represent an instance of an interface (INode). It would be beneficial to add more meaningful names to variables likecurrent_index,loop_answer_data, andarray. -
Index Initialization: Ensure that
current_indexstarts at 0 before entering the loop. -
Time Measurement: The calculation of
run_timeshould only occur after all iterations have been completed, otherwise, it will always show the elapsed time since the start of the node execution rather than just the runtime for each iteration. -
Loop Iteration: If you expect multiple passes over the same data, ensure that
numberrepresents the number of times you want to iterate over the entirearray. Otherwise, if you pass innumber=1, it might lead to unexpected behavior depending on the context.
Here's a revised version of the function with these considerations:
def loop(workflow_manage_new_instance, node: INode, generate_loop):
# Initialize necessary contexts
current_index = 0
loop_answer_data = []
index_key = "index"
item_key = "item"
# Calculate start time once before entering the loop
start_time = time.time()
# Main loop
while current_index < len(array):
# Process data using current item
currentItem = array[current_index]
# Generate output for current iteration
loop_output = generate_loop(currentItem)
loop_answer_data.append(loop_output)
# Update indices
node.context[index_key] = current_index
node.context[item_key] = currentItem
# Increment current index
current_index += 1
# Calculate total run time after all iterations
node.context['run_time'] = time.time() - start_time
def get_write_context(loop_type, array, number, loop_body):
# Implement logic to write context based on loop type
passKey Changes:
- Added comments for clarity.
- Initialized
current_indexto 0 before the loop. - Moved time measurement outside of the loop to calculate the total time taken for processing the array.
- Ensured consistent naming conventions (e.g., renaming
itemtocurrent_item) throughout the function.
… displayed incorrectly
What this PR does / why we need it?
Summary of your change
Please indicate you've done the following: