Skip to content

Commit

Permalink
Fix various bugs in dag/steps decorator (#1221)
Browse files Browse the repository at this point in the history
**Pull Request Checklist**
* Fixes #1162
* Fixes #1171
* Fixes #1223
* Fixes #1218
* Fixes #1219 
* Fixes #1222
* Fixes #1170

- [x] Tests added
- [x] Documentation/examples added
- [x] [Good commit messages](https://cbea.ms/git-commit/) and/or PR
title

**Description of PR**
Various syntax-focused fixes from the issue, including (by commit order)
* Function inputs for new-decorator functions can have no inputs, or one
input which must be a subclass of the special `Input` class, anything
else now raises an error
* DAG Task/Step names no longer use underscores
* Passing Input in the kwargs to a template call within a DAG function
will now error (instead of silently ignoring)
* Block use of new script decorator without special `Input` class
* Add ArtifactLoaders to example where needed
* Don't add a `path` for Steps/DAG artifact inputs (which would be a
lint error)
* Ignore Step/Task kwargs to allow local running
* Keep Parameter/Artifact details for Steps/DAG outputs when hoisting
output
* Fix incorrect returns for Steps/DAG
* Do not allow setting `result` or `exit_code` for Steps/DAG
* Fix adding Steps/DAGs to TemplateSets

---------

Signed-off-by: Elliot Gunton <[email protected]>
  • Loading branch information
elliotgunton authored Oct 3, 2024
1 parent 7b7a8c7 commit 1b0cd7e
Show file tree
Hide file tree
Showing 30 changed files with 634 additions and 242 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
from typing_extensions import Annotated

from hera.shared import global_config
from hera.workflows import Artifact, Input, Output, Workflow
from hera.workflows import Artifact, ArtifactLoader, Input, Output, Workflow

global_config.experimental_features["decorator_syntax"] = True

Expand All @@ -24,8 +24,8 @@


class ConcatInput(Input):
word_a: Annotated[str, Artifact(name="word_a")]
word_b: Annotated[str, Artifact(name="word_b")]
word_a: Annotated[str, Artifact(name="word_a", loader=ArtifactLoader.json)]
word_b: Annotated[str, Artifact(name="word_b", loader=ArtifactLoader.json)]


@w.script()
Expand Down Expand Up @@ -105,23 +105,21 @@
name: word_a
- from: '{{inputs.artifacts.artifact_b}}'
name: word_b
name: concat_1
name: concat-1
template: concat
- arguments:
artifacts:
- from: '{{tasks.concat_1.outputs.artifacts.an-artifact}}'
- from: '{{tasks.concat-1.outputs.artifacts.an-artifact}}'
name: word_a
- from: '{{tasks.concat_1.outputs.artifacts.an-artifact}}'
- from: '{{tasks.concat-1.outputs.artifacts.an-artifact}}'
name: word_b
depends: concat_1
depends: concat-1
name: concat-2-custom-name
template: concat
inputs:
artifacts:
- name: artifact_a
path: /tmp/hera-inputs/artifacts/artifact_a
- name: artifact_b
path: /tmp/hera-inputs/artifacts/artifact_b
name: worker
outputs:
artifacts:
Expand Down
38 changes: 19 additions & 19 deletions docs/examples/workflows/experimental/new_dag_decorator_inner_dag.md
Original file line number Diff line number Diff line change
Expand Up @@ -123,34 +123,34 @@
source: '{{inputs.parameters}}'
- dag:
tasks:
- name: setup_task
- name: setup-task
template: setup
- arguments:
parameters:
- name: word_a
value: '{{inputs.parameters.value_a}}'
- name: word_b
value: '{{tasks.setup_task.outputs.parameters.environment_parameter}}'
depends: setup_task
name: task_a
value: '{{tasks.setup-task.outputs.parameters.environment_parameter}}'
depends: setup-task
name: task-a
template: concat
- arguments:
parameters:
- name: word_a
value: '{{inputs.parameters.value_b}}'
- name: word_b
value: '{{tasks.setup_task.outputs.result}}'
depends: setup_task
name: task_b
value: '{{tasks.setup-task.outputs.result}}'
depends: setup-task
name: task-b
template: concat
- arguments:
parameters:
- name: word_a
value: '{{tasks.task_a.outputs.result}}'
value: '{{tasks.task-a.outputs.result}}'
- name: word_b
value: '{{tasks.task_b.outputs.result}}'
depends: task_a && task_b
name: final_task
value: '{{tasks.task-b.outputs.result}}'
depends: task-a && task-b
name: final-task
template: concat
inputs:
parameters:
Expand All @@ -161,7 +161,7 @@
parameters:
- name: value
valueFrom:
parameter: '{{tasks.final_task.outputs.result}}'
parameter: '{{tasks.final-task.outputs.result}}'
- dag:
tasks:
- arguments:
Expand All @@ -170,24 +170,24 @@
value: dag_a
- name: value_b
value: '{{inputs.parameters.value_a}}'
name: sub_dag_a
name: sub-dag-a
template: worker
- arguments:
parameters:
- name: value_a
value: dag_b
- name: value_b
value: '{{inputs.parameters.value_b}}'
name: sub_dag_b
name: sub-dag-b
template: worker
- arguments:
parameters:
- name: value_a
value: '{{tasks.sub_dag_a.outputs.parameters.value}}'
value: '{{tasks.sub-dag-a.outputs.parameters.value}}'
- name: value_b
value: '{{tasks.sub_dag_b.outputs.parameters.value}}'
depends: sub_dag_a && sub_dag_b
name: sub_dag_c
value: '{{tasks.sub-dag-b.outputs.parameters.value}}'
depends: sub-dag-a && sub-dag-b
name: sub-dag-c
template: worker
inputs:
parameters:
Expand All @@ -198,6 +198,6 @@
parameters:
- name: value
valueFrom:
parameter: '{{tasks.sub_dag_c.outputs.parameters.value}}'
parameter: '{{tasks.sub-dag-c.outputs.parameters.value}}'
```

40 changes: 22 additions & 18 deletions docs/examples/workflows/experimental/new_dag_decorator_params.md
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@

class SetupOutput(Output):
environment_parameter: str
an_annotated_parameter: Annotated[int, Parameter(name="dummy-param")] # use an annotated non-str
an_annotated_parameter: Annotated[int, Parameter()] # use an annotated non-str, infer name from field
setup_config: Annotated[SetupConfig, Parameter(name="setup-config")] # use a pydantic BaseModel


Expand Down Expand Up @@ -71,7 +71,8 @@


class WorkerOutput(Output):
value: str
result_value: str
another_value: str


@w.set_entrypoint
Expand All @@ -87,7 +88,7 @@
task_b = concat(ConcatInput(word_a=worker_input.value_b, word_b=setup_task.result))
final_task = concat(ConcatInput(word_a=task_a.result, word_b=task_b.result))

return WorkerOutput(value=final_task.result)
return WorkerOutput(result_value=final_task.result, another_value=setup_task.an_annotated_parameter)
```

=== "YAML"
Expand All @@ -106,9 +107,9 @@
- name: environment_parameter
valueFrom:
path: /tmp/hera-outputs/parameters/environment_parameter
- name: dummy-param
- name: an_annotated_parameter
valueFrom:
path: /tmp/hera-outputs/parameters/dummy-param
path: /tmp/hera-outputs/parameters/an_annotated_parameter
- name: setup-config
valueFrom:
path: /tmp/hera-outputs/parameters/setup-config
Expand Down Expand Up @@ -156,40 +157,40 @@
source: '{{inputs.parameters}}'
- dag:
tasks:
- name: setup_task
- name: setup-task
template: setup
- arguments:
parameters:
- name: word_a
value: '{{inputs.parameters.value_a}}'
- name: word_b
value: '{{tasks.setup_task.outputs.parameters.environment_parameter}}{{tasks.setup_task.outputs.parameters.dummy-param}}'
value: '{{tasks.setup-task.outputs.parameters.environment_parameter}}{{tasks.setup-task.outputs.parameters.an_annotated_parameter}}'
- name: concat_config
value: '{"reverse": false}'
depends: setup_task
name: task_a
depends: setup-task
name: task-a
template: concat
- arguments:
parameters:
- name: word_a
value: '{{inputs.parameters.value_b}}'
- name: word_b
value: '{{tasks.setup_task.outputs.result}}'
value: '{{tasks.setup-task.outputs.result}}'
- name: concat_config
value: '{"reverse": false}'
depends: setup_task
name: task_b
depends: setup-task
name: task-b
template: concat
- arguments:
parameters:
- name: word_a
value: '{{tasks.task_a.outputs.result}}'
value: '{{tasks.task-a.outputs.result}}'
- name: word_b
value: '{{tasks.task_b.outputs.result}}'
value: '{{tasks.task-b.outputs.result}}'
- name: concat_config
value: '{"reverse": false}'
depends: task_a && task_b
name: final_task
depends: task-a && task-b
name: final-task
template: concat
inputs:
parameters:
Expand All @@ -203,8 +204,11 @@
name: worker
outputs:
parameters:
- name: value
- name: result_value
valueFrom:
parameter: '{{tasks.final_task.outputs.result}}'
parameter: '{{tasks.final-task.outputs.result}}'
- name: another_value
valueFrom:
parameter: '{{tasks.setup-task.outputs.parameters.an_annotated_parameter}}'
```

Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@
clusterScope: true
name: my-cluster-workflow-template
template: run-setup-dag
- name: setup_task
- name: setup-task
templateRef:
clusterScope: true
name: my-cluster-workflow-template
Expand All @@ -126,11 +126,11 @@
- name: word_a
value: '{{inputs.parameters.value_a}}'
- name: word_b
value: '{{tasks.setup_task.outputs.parameters.environment_parameter}}{{tasks.setup_task.outputs.parameters.dummy-param}}'
value: '{{tasks.setup-task.outputs.parameters.environment_parameter}}{{tasks.setup-task.outputs.parameters.dummy-param}}'
- name: concat_config
value: '{"reverse": false}'
depends: setup_task
name: task_a
depends: setup-task
name: task-a
templateRef:
name: my-workflow-template
template: concat
Expand All @@ -139,24 +139,24 @@
- name: word_a
value: '{{inputs.parameters.value_b}}'
- name: word_b
value: '{{tasks.setup_task.outputs.result}}'
value: '{{tasks.setup-task.outputs.result}}'
- name: concat_config
value: '{"reverse": false}'
depends: setup_task
name: task_b
depends: setup-task
name: task-b
templateRef:
name: my-workflow-template
template: concat
- arguments:
parameters:
- name: word_a
value: '{{tasks.task_a.outputs.result}}'
value: '{{tasks.task-a.outputs.result}}'
- name: word_b
value: '{{tasks.task_b.outputs.result}}'
value: '{{tasks.task-b.outputs.result}}'
- name: concat_config
value: '{"reverse": false}'
depends: task_a && task_b
name: final_task
depends: task-a && task-b
name: final-task
templateRef:
name: my-workflow-template
template: concat
Expand All @@ -174,6 +174,6 @@
parameters:
- name: value
valueFrom:
parameter: '{{tasks.final_task.outputs.result}}'
parameter: '{{tasks.final-task.outputs.result}}'
```

Original file line number Diff line number Diff line change
Expand Up @@ -138,33 +138,33 @@
parameters:
- name: value
valueFrom:
parameter: '{{steps.final_step.outputs.result}}'
parameter: '{{steps.final-step.outputs.result}}'
steps:
- - name: setup_step
- - name: setup-step
template: setup
- - arguments:
parameters:
- name: word_a
value: '{{inputs.parameters.value_a}}'
- name: word_b
value: '{{steps.setup_step.outputs.parameters.environment_parameter}}{{steps.setup_step.outputs.parameters.dummy-param}}'
name: step_a
value: '{{steps.setup-step.outputs.parameters.environment_parameter}}{{steps.setup-step.outputs.parameters.dummy-param}}'
name: step-a
template: concat
- arguments:
parameters:
- name: word_a
value: '{{inputs.parameters.value_b}}'
- name: word_b
value: '{{steps.setup_step.outputs.result}}'
name: step_b
value: '{{steps.setup-step.outputs.result}}'
name: step-b
template: concat
- - arguments:
parameters:
- name: word_a
value: '{{steps.step_a.outputs.result}}'
value: '{{steps.step-a.outputs.result}}'
- name: word_b
value: '{{steps.step_b.outputs.result}}'
name: final_step
value: '{{steps.step-b.outputs.result}}'
name: final-step
template: concat
```

13 changes: 13 additions & 0 deletions docs/examples/workflows/experimental/template_sets.md
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,12 @@
return Output(result="Setting things up")


@templates.dag()
def my_dag():
setup(name="task-a")
setup(name="task-b")


w.add_template_set(templates)
```

Expand Down Expand Up @@ -52,5 +58,12 @@
value: ''
image: python:3.9
source: '{{inputs.parameters}}'
- dag:
tasks:
- name: task-a
template: setup
- name: task-b
template: setup
name: my-dag
```

Loading

0 comments on commit 1b0cd7e

Please sign in to comment.