Skip to content
This repository has been archived by the owner on Oct 9, 2023. It is now read-only.

Fixing recovering of SKIPPED nodes #551

Merged
merged 1 commit into from
Apr 7, 2023
Merged

Conversation

hamersaw
Copy link
Contributor

@hamersaw hamersaw commented Apr 7, 2023

TL;DR

The PR to enable recovering dynamic workflows when they have already compiled the workflow closure updated the node executor to always use the phase returned from attemptRecovery. This surfaced an issue where nodes in the SKIPPED phase were reported as SKIPPED during recovery when they should be executed in the recovered workflow.

This PR correctly reports the phase of previously SKIPPED nodes as UNDEFINED to ensure the nodes are correctly evaluated during workflow recoveries.

Type

  • Bug Fix
  • Feature
  • Plugin

Are all requirements met?

  • Code completed
  • Smoke tested
  • Unit tests added
  • Code documentation added
  • Any pending items have an associated Issue

Complete description

^^^

Tracking Issue

fixes flyteorg/flyte#3578

Follow-up issue

NA

@hamersaw hamersaw requested review from kumare3 and EngHabu as code owners April 7, 2023 21:29
@codecov
Copy link

codecov bot commented Apr 7, 2023

Codecov Report

Merging #551 (4ca0571) into master (986f014) will increase coverage by 0.41%.
The diff coverage is 0.00%.

❗ Current head 4ca0571 differs from pull request most recent head f1ca4b6. Consider uploading reports for the commit f1ca4b6 to get more accurate results

@hamersaw hamersaw merged commit 7abdcd7 into master Apr 7, 2023
@hamersaw hamersaw deleted the bug/recovering-skipped-nodes branch April 7, 2023 22:12
eapolinario pushed a commit to eapolinario/flytepropeller that referenced this pull request Aug 9, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
2 participants