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

Cannot use inline runner with Pydantic types? #1245

Open
3 of 7 tasks
alicederyn opened this issue Oct 21, 2024 · 1 comment
Open
3 of 7 tasks

Cannot use inline runner with Pydantic types? #1245

alicederyn opened this issue Oct 21, 2024 · 1 comment
Labels
type:bug A general bug

Comments

@alicederyn
Copy link
Collaborator

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. I have searched for existing issues

  • Yes

3. This bug occurs in Hera when...

  • exporting to YAML
  • submitting to Argo
  • running on Argo with the Hera runner
  • other:

Bug report

Describe the bug
The inline runner code generated for Pydantic IO functions is illegal. For example, I'm seeing the following error:

Traceback (most recent call last):
  File "/argo/staging/script", line 9, in <module>
    print(input.message)
          ^^^^^^^^^^^^^
AttributeError: 'builtin_function_or_method' object has no attribute 'message'

To Reproduce
Full Hera code to reproduce the bug:

from hera.shared import global_config
from hera.workflows import Input, Steps, Workflow, script

global_config.experimental_features["script_pydantic_io"] = True

class EchoInput(Input):
    """Inputs to the echo function."""

    # TODO Remove class or replace with your inputs
    message: Annotated[str, Parameter(description="A message to print")]

@script(constructor="inline")
def echo(input: EchoInput) -> None:
    print(input.message)  ## ERROR IS HERE

with Workflow(name="my-workflow") as w:
    with Steps(name="steps"):
        echo{arguments={"message": "Hello world"})

Expected behavior
Either an error when generating the YAML that inline mode is not compatible with Pydantic IO, or valid generated Python.

Environment

  • Hera Version: 5.17.1
  • Python Version: 3.11.8
  • Argo Version: 3.4.17.1

Additional context
The source of the problem is obvious when looking at the YAML:

      source: |-
        import os
        import sys
        sys.path.append(os.getcwd())
        import json
        try: message = json.loads(r'''{{inputs.parameters.message}}''')
        except: message = r'''{{inputs.parameters.message}}'''

        """Prints out a message."""
        print(input.message)

It would be very helpful for teaching purposes if at least a subset of the Pydantic IO features could be supported with the inline runner. In our setup, changing and deploying a non-inline script requires pushing a commit to a PR and waiting for CI, while inline scripts can be modified and tested with a simpler, local, make-based flow. This would probably require introspecting the Pydantic types and generating equivalent classes in the generated code.

Failing that, a more explanatory failure mode would be valuable for users migrating old flows to Pydantic IO.

@alicederyn alicederyn added the type:bug A general bug label Oct 21, 2024
@elliotgunton
Copy link
Collaborator

I would rather not start supporting a subset of features for something that's explicitly not for inline scripts.

https://hera.readthedocs.io/en/stable/user-guides/script-runner-io/

Hera provides the Input and Output Pydantic classes which can be used to more succinctly write your script function inputs and outputs, and requires use of the Hera Runner.

We could add an error, but I'm also concerned about increasing the complexity of the codebase for all the handholding for user errors.

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

No branches or pull requests

2 participants