From 8a764a98eb140de2111e87212b6d5dd6302a2d31 Mon Sep 17 00:00:00 2001 From: Alice Date: Tue, 13 Aug 2024 15:16:59 +0100 Subject: [PATCH] Disable template name conflict detection (#1153) **Pull Request Checklist** - [X] Fixes #1151 - [X] Tests added - [X] Documentation/examples added - [x] [Good commit messages](https://cbea.ms/git-commit/) and/or PR title **Description of PR** Currently, template name conflict detection is causing a stack overflow when run on a recursive template. This PR removes template name conflict detection, which was added in #1054. --------- Signed-off-by: Alice Purcell --- examples/workflows/steps/recursive-steps.yaml | 33 +++++++++++++++ examples/workflows/steps/recursive_steps.py | 22 ++++++++++ src/hera/workflows/_context.py | 4 +- src/hera/workflows/exceptions.py | 2 +- tests/test_unit/test_context.py | 40 +++++++++++-------- 5 files changed, 80 insertions(+), 21 deletions(-) create mode 100644 examples/workflows/steps/recursive-steps.yaml create mode 100644 examples/workflows/steps/recursive_steps.py diff --git a/examples/workflows/steps/recursive-steps.yaml b/examples/workflows/steps/recursive-steps.yaml new file mode 100644 index 000000000..caaf3b4f9 --- /dev/null +++ b/examples/workflows/steps/recursive-steps.yaml @@ -0,0 +1,33 @@ +apiVersion: argoproj.io/v1alpha1 +kind: WorkflowTemplate +metadata: + name: my-workflow +spec: + entrypoint: steps + templates: + - name: sub-steps + steps: + - - name: random-roll + template: random-roll + - - arguments: + parameters: + - name: input-num + value: '{{steps.random-roll.outputs.result}}' + name: recurse + template: sub-steps + when: '{{steps.random-roll.outputs.result}} != 0' + - name: random-roll + script: + command: + - python + image: python:3.8 + source: |- + import os + import sys + sys.path.append(os.getcwd()) + import random + return random.randint(0, 2) + - name: steps + steps: + - - name: sub-steps + template: sub-steps diff --git a/examples/workflows/steps/recursive_steps.py b/examples/workflows/steps/recursive_steps.py new file mode 100644 index 000000000..abc56d02c --- /dev/null +++ b/examples/workflows/steps/recursive_steps.py @@ -0,0 +1,22 @@ +from hera.workflows import Step, Steps, WorkflowTemplate, script + + +@script(constructor="inline") +def random_roll(): + import random + + return random.randint(0, 2) + + +with WorkflowTemplate(name="my-workflow", entrypoint="steps") as w: + with Steps(name="sub-steps") as sub_steps: + random_number = random_roll() + Step( + name="recurse", + arguments={"input-num": random_number.result}, + template=sub_steps, + when=f"{random_number.result} != 0", + ) + + with Steps(name="steps"): + sub_steps() diff --git a/src/hera/workflows/_context.py b/src/hera/workflows/_context.py index bc70ba0ca..b7f4b07a9 100644 --- a/src/hera/workflows/_context.py +++ b/src/hera/workflows/_context.py @@ -8,7 +8,7 @@ from typing import List, Optional, TypeVar, Union from hera.shared import BaseMixin -from hera.workflows.exceptions import InvalidType, TemplateNameConflict +from hera.workflows.exceptions import InvalidType from hera.workflows.protocol import Subbable, TTemplate TNode = TypeVar("TNode", bound="SubNodeMixin") @@ -102,8 +102,6 @@ def add_sub_node(self, node: Union[SubNodeMixin, TTemplate]) -> None: found = False for t in pieces[0].templates: if t.name == node.template.name: - if t != node.template: - raise TemplateNameConflict(f"Found multiple templates with the same name: {t.name}") found = True break if not found: diff --git a/src/hera/workflows/exceptions.py b/src/hera/workflows/exceptions.py index f39548513..74c56f6d6 100644 --- a/src/hera/workflows/exceptions.py +++ b/src/hera/workflows/exceptions.py @@ -26,7 +26,7 @@ class InvalidDispatchType(WorkflowsException): class TemplateNameConflict(WorkflowsException): - """Exception raised when multiple Templates are found with the same name..""" + """Currently unused.""" ... diff --git a/tests/test_unit/test_context.py b/tests/test_unit/test_context.py index 859f110a4..710146aba 100644 --- a/tests/test_unit/test_context.py +++ b/tests/test_unit/test_context.py @@ -1,7 +1,7 @@ import pytest -from hera.workflows import DAG, Steps, WorkflowTemplate, script -from hera.workflows.exceptions import NodeNameConflict, TemplateNameConflict +from hera.workflows import DAG, Step, Steps, WorkflowTemplate, script +from hera.workflows.exceptions import NodeNameConflict class TestContextNameConflicts: @@ -10,24 +10,30 @@ class TestContextNameConflicts: and that no two Task/Step nodes have the same name. """ - def test_conflict_on_templates_with_same_name(self): - """Multiple templates can't have the same name.""" - name = "name-of-dag-and-script" + def test_allows_recursive_steps(self): + """Recursive steps should not cause an error. - @script(name=name) - def example(): - print("hello") + See https://github.com/argoproj-labs/hera/issues/1151 + """ - with pytest.raises(TemplateNameConflict): - with WorkflowTemplate(name="my-workflow", entrypoint=name), DAG(name=name): - example() + @script(constructor="inline") + def random_roll(): + import random - with pytest.raises(TemplateNameConflict): - with WorkflowTemplate( - name="my-workflow", - entrypoint=name, - ), Steps(name=name): - example() + return random.randint(0, 2) + + with WorkflowTemplate(name="my-workflow", entrypoint="steps"): + with Steps(name="sub-steps") as st: + random_number = random_roll() + Step( + name="recurse", + arguments={"input-num": random_number.result}, + template=st, + when=f"{random_number.result} != 0", + ) + + with Steps(name="steps"): + st() def test_no_conflict_on_tasks_with_different_names_using_same_template(self): """Task nodes can have different names for the same script template."""