Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

New dag/steps decorators issues/inconsistencies #1162

Open
7 tasks done
elliotgunton opened this issue Aug 20, 2024 · 1 comment · May be fixed by #1221
Open
7 tasks done

New dag/steps decorators issues/inconsistencies #1162

elliotgunton opened this issue Aug 20, 2024 · 1 comment · May be fixed by #1221
Assignees
Labels
type:bug A general bug

Comments

@elliotgunton
Copy link
Collaborator

elliotgunton commented Aug 20, 2024

Bug report

Describe the bug
A clear and concise description of what the bug is:

There are a few issues with the new dag decorator.

  • It doesn't accept artifacts because of the path existing in the yaml. I'm not sure if DAGs can even accept artifacts in the first place if they are used as tasks themselves. Tried submitting examples/workflows/experimental/new_dag_decorator_artifacts.py but got

Server returned status code 400 with message: templates.worker.inputs.artifacts.artifact_a.path only valid in container/script templates

  • The mandated Input for a DAG function doesn't really make sense in the current form - if I don't have any inputs I still need one due to the check here
    if len(func_inputs) == 1:

i.e. my function must be:

@w.dag()
def my_dag(_: Input):
  • The Output is not mandated but you can e.g. set result or exit_code on it which should probably be an error within a DAG/Steps function.

  • DAG tasks must have alphanumeric hyphenated names, meaning the python variable names can't be directly used via varname. E.g for the code:

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

global_config.experimental_features["decorator_syntax"] = True


w = Workflow(generate_name="my-workflow-")


class MyInput(Input):
    user: str


@w.script()
def hello_world(my_input: MyInput) -> Output:
    output = Output()
    output.result = f"Hello Hera User: {my_input.user}!"
    return output


@w.set_entrypoint
@w.dag()
def worker(_: Input):
    task_name = hello_world(my_input=MyInput(user="test"))

argo lint basic-dag.yaml gets

argo lint manifests/basic-dag.yaml
manifests/basic-dag.yaml:
   ✖ in "my-workflow-" (Workflow): templates.worker.tasks[0].name: 'task_name' is invalid: name must consist of alpha-numeric characters or '-', and must start with an alpha-numeric character (e.g. My-name1-2, 123-NAME)

✖ 1 linting errors found!
  • A result of the above is local running doesn't work when using alternative names as task kwargs aren't accepted:
python -m hera_demo.advanced_dag
Traceback (most recent call last):
  File "<frozen runpy>", line 198, in _run_module_as_main
  File "<frozen runpy>", line 88, in _run_code
  File "/Users/elliot/projects/hera-demo/hera_demo/advanced_dag.py", line 97, in <module>
    worker(WorkerInput(value_b="running locally!"))
  File "/Users/elliot/Library/Caches/pypoetry/virtualenvs/hera-example-woe73TMe-py3.12/lib/python3.12/site-packages/hera/workflows/_meta_mixins.py", line 814, in call_wrapper
    return func(*args, **kwargs)
           ^^^^^^^^^^^^^^^^^^^^^
  File "/Users/elliot/projects/hera-demo/hera_demo/advanced_dag.py", line 74, in worker
    setup_task = setup(name="setup-task")
                 ^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/elliot/Library/Caches/pypoetry/virtualenvs/hera-example-woe73TMe-py3.12/lib/python3.12/site-packages/hera/workflows/_meta_mixins.py", line 674, in script_call_wrapper
    return func(*args, **kwargs)
           ^^^^^^^^^^^^^^^^^^^^^
TypeError: setup() got an unexpected keyword argument 'name'
  • In the DAG function, specifying inputs to function calls with kwargs fails silently, i.e. doing
@w.set_entrypoint
@w.dag()
def worker(_: Input):
    hello_world(my_input=MyInput(user="test"))

Creates a YAML with no arguments to the task:

  - dag:
      tasks:
      - name: hello-world
        template: hello-world
    name: worker

This is by design as kwargs are reserved for Task kwargs, but we could be more helpful.

  • Artifact passing examples need ArtifactLoaders (file type), otherwise for this code:
@w.dag()
def worker(_: Input) -> ArtifactOutput:
    artifact_task = create_artifact(name="artifact-task")
    concat_1 = concat(
        ConcatInput(
            word_a=artifact_task.an_artifact,
            word_b=artifact_task.an_artifact,
        ),
        name="concat-1",
    )
    return ArtifactOutput(an_artifact=concat_1.an_artifact)

concat-1 outputs an artifact of the literal string "/tmp/hera-inputs/artifacts/word_a /tmp/hera-inputs/artifacts/word_b"

@elliotgunton
Copy link
Collaborator Author

For

The Output is not mandated but you can e.g. set result or exit_code on it which should probably be an error within a DAG/Steps function.

and

local running doesn't work when using alternative names as task kwargs aren't accepted

These seem a bit harder (checking context at runtime for value access) so will try in new issues/PRs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type:bug A general bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant