Skip to content

Commit

Permalink
Disable template name conflict detection (#1153)
Browse files Browse the repository at this point in the history
**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 <alicederyn@gmail.com>
  • Loading branch information
alicederyn authored Aug 13, 2024
1 parent ce91207 commit 8a764a9
Show file tree
Hide file tree
Showing 5 changed files with 80 additions and 21 deletions.
33 changes: 33 additions & 0 deletions examples/workflows/steps/recursive-steps.yaml
Original file line number Diff line number Diff line change
@@ -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
22 changes: 22 additions & 0 deletions examples/workflows/steps/recursive_steps.py
Original file line number Diff line number Diff line change
@@ -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()
4 changes: 1 addition & 3 deletions src/hera/workflows/_context.py
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down Expand Up @@ -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:
Expand Down
2 changes: 1 addition & 1 deletion src/hera/workflows/exceptions.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ class InvalidDispatchType(WorkflowsException):


class TemplateNameConflict(WorkflowsException):
"""Exception raised when multiple Templates are found with the same name.."""
"""Currently unused."""

...

Expand Down
40 changes: 23 additions & 17 deletions tests/test_unit/test_context.py
Original file line number Diff line number Diff line change
@@ -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:
Expand All @@ -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."""
Expand Down

0 comments on commit 8a764a9

Please sign in to comment.