Skip to content

Commit fe781b4

Browse files
committed
Fix nested conditionals running skipped branches
In local execution, a conditional nested inside the active branch of another conditional (inline, or inside a subworkflow called from that branch) reused the parent's execution state, which was already marked BRANCH_ACTIVE. Every branch of the nested conditional therefore ran its tasks, even the ones whose condition was false. Give every local conditional section its own execution state starting in BRANCH_SKIPPED, so only the selected branch of a nested conditional runs. Sections nested in a skipped branch still become SkippedConditionalSection, as before. Closes flyteorg/flyte#6426 Signed-off-by: Tianlang (Jackie) Chen <305351710+jackiectl2@users.noreply.github.com>
1 parent eb22787 commit fe781b4

2 files changed

Lines changed: 94 additions & 9 deletions

File tree

‎flytekit/core/context_manager.py‎

Lines changed: 7 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -838,15 +838,13 @@ def enter_conditional_section(self) -> FlyteContext.Builder:
838838

839839
if self.execution_state:
840840
if self.execution_state.is_local_execution():
841-
if self.in_a_condition:
842-
if self.execution_state.branch_eval_mode == BranchEvalMode.BRANCH_SKIPPED:
843-
self.execution_state = self.execution_state.with_params()
844-
else:
845-
# In case of local workflow execution we should ensure a conditional section
846-
# is created so that skipped branches result in tasks not being executed
847-
self.execution_state = self.execution_state.with_params(
848-
branch_eval_mode=BranchEvalMode.BRANCH_SKIPPED
849-
)
841+
# In case of local workflow execution we should ensure a conditional section
842+
# is created so that skipped branches result in tasks not being executed.
843+
# This also applies to nested conditionals (inline, or inside a subworkflow called
844+
# from an active branch): they must not inherit the parent's active branch state.
845+
self.execution_state = self.execution_state.with_params(
846+
branch_eval_mode=BranchEvalMode.BRANCH_SKIPPED
847+
)
850848

851849
self.in_a_condition = True
852850
return self

‎tests/flytekit/unit/core/test_conditions.py‎

Lines changed: 87 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -494,6 +494,93 @@ def multiplier_2(my_input: float) -> float:
494494
assert res == 20
495495

496496

497+
def test_nested_condition_runs_only_selected_branch():
498+
executed = []
499+
500+
@task
501+
def inner_if(n: float) -> float:
502+
executed.append("inner_if")
503+
return n
504+
505+
@task
506+
def inner_else(n: float) -> float:
507+
executed.append("inner_else")
508+
return -n
509+
510+
@task
511+
def outer_else(n: float) -> float:
512+
executed.append("outer_else")
513+
return 0.0
514+
515+
@workflow
516+
def wf(my_input: float) -> float:
517+
return (
518+
conditional("outer")
519+
.if_(my_input > 0.1)
520+
.then(
521+
conditional("inner")
522+
.if_(my_input < 0.5)
523+
.then(inner_if(n=my_input))
524+
.else_()
525+
.then(inner_else(n=my_input))
526+
)
527+
.else_()
528+
.then(outer_else(n=my_input))
529+
)
530+
531+
assert wf(my_input=0.6) == -0.6
532+
assert executed == ["inner_else"]
533+
534+
executed.clear()
535+
assert wf(my_input=0.3) == 0.3
536+
assert executed == ["inner_if"]
537+
538+
executed.clear()
539+
assert wf(my_input=0.0) == 0.0
540+
assert executed == ["outer_else"]
541+
542+
543+
def test_condition_in_subworkflow_called_from_branch():
544+
# https://github.com/flyteorg/flyte/issues/6426
545+
executed = []
546+
547+
@task
548+
def do_test() -> bool:
549+
executed.append("do_test")
550+
return False
551+
552+
@task
553+
def do_nothing1() -> bool:
554+
executed.append("do_nothing1")
555+
return True
556+
557+
@task
558+
def do_nothing2() -> bool:
559+
executed.append("do_nothing2")
560+
return True
561+
562+
@task
563+
def do_nothing3() -> bool:
564+
executed.append("do_nothing3")
565+
return True
566+
567+
@workflow
568+
def sub_workflow() -> bool:
569+
some_test = do_test()
570+
return conditional("inner").if_(some_test.is_true()).then(do_nothing1()).else_().then(do_nothing2())
571+
572+
@workflow
573+
def top_workflow(input_value: bool) -> bool:
574+
return conditional("outer").if_(input_value.is_true()).then(sub_workflow()).else_().then(do_nothing3()) # type: ignore
575+
576+
assert top_workflow(input_value=True) is True
577+
assert executed == ["do_test", "do_nothing2"]
578+
579+
executed.clear()
580+
assert top_workflow(input_value=False) is True
581+
assert executed == ["do_nothing3"]
582+
583+
497584
def test_echo_in_condition():
498585
echo1 = Echo(name="echo", inputs={"a": typing.Optional[float]})
499586

0 commit comments

Comments
 (0)