argoproj-labs / hera

Hera makes Python code easy to orchestrate on Argo Workflows through native Python integrations. It lets you construct and submit your Workflows entirely in Python. ⭐️ Remember to star!
https://hera.rtfd.io
Apache License 2.0
560 stars 105 forks source link

Recursive YAML generation fail (hera 5.16.1) #1151

Closed gcemaj closed 1 month ago

gcemaj commented 1 month ago

Pre-bug-report checklist

1. This bug can be reproduced using pure Argo YAML

If yes, it is more likely to be an Argo bug unrelated to Hera. Please double check before submitting an issue to Hera.

2. This bug occurs in Hera when...

Bug report

Describe the bug Cannot generate YAML when building recursive steps, this works in hera <= 5.14.*

Error log if applicable:

  File "/.../hera/workflows/_meta_mixins.py", line 376, in __call__
    return Step(template=self, **kwargs)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/.../pydantic/v1/main.py", line 349, in __init__
    __pydantic_self__._init_private_attributes()
  File "/.../hera/shared/_global_config.py", line 181, in _init_private_attributes
    self.__hera_init__()
  File "/.../hera/workflows/_context.py", line 30, in __hera_init__
    _context.add_sub_node(self)
  File "/.../hera/workflows/_context.py", line 105, in add_sub_node
    if t != node.template:
   .
   .
   .
   .
Maximum depth exceeded...

To Reproduce Full Hera code to reproduce the bug:

from hera.expr import g
from hera.workflows import Parameter, Steps, WorkflowTemplate, Step

@script(constructor="inline")
def do(input_num: Annotated[int, Parameter(description="a number")]) -> Annotated[int, Parameter(name="output-num")]:
    return input_num - 1

def get_workflow_template() -> WorkflowTemplate:
    with WorkflowTemplate(
        name="<name>",
        entrypoint="steps",
    ) as wt:
        with Steps(name="sub-steps", inputs=[Parameter(name="input-num", value=5)]) as st:
            do_output = do(arguments={"input-num": f"{g.inputs.parameters.get('input-num'):$}"})
            Step(
                name="recurse",
                arguments={
                    "input-num": do_output.get_parameter('output-num').value,
                },
                template=st,
                when=f"{do_output.get_parameter('output-num')} <= 0",
            )

        with Steps(name="steps"):
            st(
                arguments={
                    "input-num": 10
                }
            )

        return wt

Expected behavior For the YAML to be properly generated, for this minimal example it shoudl be something like

apiVersion: argoproj.io/v1alpha1
kind: WorkflowTemplate
metadata:
  name: <name>
spec:
  entrypoint: steps
  templates:
  - inputs:
      parameters:
      - name: input-num
        value: '5'
    name: sub-steps
    steps:
    - - arguments:
          parameters:
          - name: input-num
            value: '{{inputs.parameters.input-num}}'
        name: do
        template: do
    - - arguments:
          parameters:
          - name: input-num
            value: '{{steps.do.outputs.parameters.output-num}}'
        name: recurse
        template: sub-steps
        when: '{{steps.do.outputs.parameters.output-num}} <= 0'
  - inputs:
      parameters:
      - description: a number
        name: input_num
    name: do
    outputs:
      parameters:
      - name: output-num
    script:
      command:
      - python
      image: <image>
      source: |-
        import os
        import sys
        sys.path.append(os.getcwd())
        import json
        try: input_num = json.loads(r'''{{inputs.parameters.input_num}}''')
        except: input_num = r'''{{inputs.parameters.input_num}}'''

        """Prints out a message."""
        return input_num - 1
  - name: steps
    steps:
    - - arguments:
          parameters:
          - name: input-num
            value: '10'
        name: sub-steps
        template: sub-steps

Environment

Additional context Add any other context about the problem here.

alicederyn commented 1 month ago

Looks like this issue was introduced in https://github.com/argoproj-labs/hera/pull/1054 (merged in 5.16.0)