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.

Sign up for GitHub

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

Jump to bottom

Add pydantic IO docs #939

Merged
merged 7 commits into from
Feb 12, 2024
Merged

Add pydantic IO docs #939

merged 7 commits into from
Feb 12, 2024

Conversation

elliotgunton
Copy link
Collaborator

Add docs to script user guide and experimental features section

@elliotgunton elliotgunton added type:documentation A documentation update semver:minor A change requiring a minor version bump labels Jan 22, 2024
@elliotgunton elliotgunton mentioned this pull request Jan 24, 2024
Merged
4 tasks
@elliotgunton elliotgunton force-pushed the pydantic-v2-io branch 8 times, most recently from 552727e to 9bfdb1c Compare January 29, 2024 10:58
@elliotgunton elliotgunton force-pushed the pydantic-io-docs branch 2 times, most recently from fa13f8d to 826c13a Compare January 29, 2024 11:11
samj1912 pushed a commit that referenced this pull request Jan 30, 2024
@elliotgunton
Add Pydantic V1 IO models for use with Hera Runner (#920)
7998f2e 
**Pull Request Checklist**
- [x] Part of #858 
- [x] Tests added
- [ ] ~Documentation/examples added~ See #939 
- [x] [Good commit messages](https://cbea.ms/git-commit/) and/or PR
title

**Description of PR**
Currently hera i/o with annotated params can become extremely verbose.
The output syntax is especially error-prone.

This PR introduces custom Input/Output BaseModels for users to subclass,
which allow a cleaner arrangement of inputs and outputs for functions.
These are available under the `script_pydantic_io` experimental feature
flag.

With these Pydantic input/output models, the following should be noted:
* duplicated param names (for normal Parameters as well as the new
models) are now detected in Hera rather than when linted by Argo (as
well as duplicated artifact names). Parameters and Artifacts having the
same name is legal in the Argo spec as they exist in different scopes
e.g.
```yaml
...
      inputs:
        parameters:
          - name: my-name
            default: test
        artifacts:
          - name: my-name
            path: /tmp
            optional: true
...
```
* `exit_code` and `result` are reserved attributes for the
`RunnerOutput`. A user trying to use their own parameters with these
names would have to be specified with an annotated parameter e.g.
`my_exit_code: Annotated[int, Parameter(name="exit_code")]` (TBC with a
test)
* Scripts cannot have a return tuple containing any `RunnerOutput` to
avoid multiple `exit_code`s being specified. @samj1912 / @flaviuvadan
this is up for debate but I think would encourage better practices to
discourage tuples and have a single script template outputting a single
`RunnerOutput` subclass, and it keeps the logic clearer from the Hera
side. Users can still use inline output parameters alongside the
`RunnerOutput` return annotation
* Multiple input parameters when using a `RunnerInput` in the function
params **is not legal**
* A `RunnerInput`'s `__fields__` as defined by pydantic are used to
"explode" the input class into constituent parameters for the Argo spec.
i.e. using the following class as an input param to a script function:
```py
class MyInput(RunnerInput):
     my_input_str: str
     my_input_int: int

@script(constructor="runner")
def my_func(my_input: MyInput):
    ...
```
will create the script template `my_func` in yaml with Parameters
`my_input_str` and `my_input_int`, NOT `my_input`, [see the
example](https://github.com/argoproj-labs/hera/blob/92f11d341eb29d2501b9ee5be57a703160b35e24/docs/examples/workflows/experimental/script_pydantic_io.md)

---------

Signed-off-by: Elliot Gunton <egunton@bloomberg.net>
@elliotgunton elliotgunton force-pushed the pydantic-v2-io branch 3 times, most recently from cc03607 to 4ded421 Compare January 31, 2024 11:20
@elliotgunton elliotgunton force-pushed the pydantic-io-docs branch 2 times, most recently from 7941610 to d0f47a9 Compare February 7, 2024 12:30
@elliotgunton elliotgunton force-pushed the pydantic-io-docs branch 2 times, most recently from 213cbf1 to db65509 Compare February 12, 2024 12:21
Base automatically changed from pydantic-v2-io to main February 12, 2024 12:38
@samj1912 samj1912 enabled auto-merge (squash) February 12, 2024 12:39
@codecov Codecov
Copy link

codecov bot commented Feb 12, 2024 •
edited
Loading

Codecov Report

Attention: 9 lines in your changes are missing coverage. Please review.

Comparison is base (b146956) 81.8% compared to head (3b92519) 81.7%.

Files Patch % Lines
src/hera/workflows/runner.py 57.1% 8 Missing and 1 partial ⚠️
Additional details and impacted files 
@@           Coverage Diff           @@
##            main    #939     +/-   ##
=======================================
- Coverage   81.8%   81.7%   -0.2%     
=======================================
  Files         54      54             
  Lines       4188    4208     +20     
  Branches     883     889      +6     
=======================================
+ Hits        3428    3439     +11     
- Misses       566     574      +8     
- Partials     194     195      +1

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

elliotgunton and others added 6 commits February 12, 2024 12:48
@elliotgunton
Add RunnerInput/Output Pydantic V2 classes
b56c2a8 
* Enables users to use Pydantic V2 objects in their scripts while
maintaining V1 usage internally for Hera
* RunnerInput/Output classes are defined in hera.workflows.io.v1 and
hera.workflows.io.v2, allowing users to use v1 or v2 depending on their
installation and migration progress.

Signed-off-by: Elliot Gunton <egunton@bloomberg.net>
@elliotgunton
Add __init__ file for io module
332fad2 
Signed-off-by: Elliot Gunton <egunton@bloomberg.net>
@elliotgunton
Add pydantic IO docs
eb52424 
Signed-off-by: Elliot Gunton <egunton@bloomberg.net>
@elliotgunton
Fix docs to use io init
0b62650 
Signed-off-by: Elliot Gunton <egunton@bloomberg.net>
@elliotgunton @samj1912
Save dummy outputs when runner script raises an exception (#949)
57f8f6f 
**Pull Request Checklist**
- [x] Fixes #948 
- [x] Tests added
- [ ] Documentation/examples added
- [x] [Good commit messages](https://cbea.ms/git-commit/) and/or PR
title

**Description of PR**
When running a script via the Hera Runner, if it raises an error, the
declared outputs of the (script) template are not created as files in
the container, leading to confusing Argo logs such as:

```
msg="cannot save parameter /tmp/hera-outputs/parameters/my-parameter"
argo=true
error="open /tmp/hera-outputs/parameters/my-parameter: no such file or directory"`
```

This PR makes the runner catch the exception, write dummy values, then
pass the exception up.

Signed-off-by: Elliot Gunton <egunton@bloomberg.net>
Co-authored-by: Sambhav Kothari <skothari44@bloomberg.net>
@elliotgunton
Assert type to fix lint error
effdd09 
Signed-off-by: Elliot Gunton <egunton@bloomberg.net>
@elliotgunton
Fix bad merge
3b92519 
Signed-off-by: Elliot Gunton <egunton@bloomberg.net>
@elliotgunton elliotgunton enabled auto-merge (squash) February 12, 2024 12:51
@elliotgunton elliotgunton merged commit 5c5b665 into main Feb 12, 2024
20 of 22 checks passed
@elliotgunton elliotgunton deleted the pydantic-io-docs branch February 12, 2024 12:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@samj1912 samj1912 samj1912 approved these changes

@flaviuvadan flaviuvadan flaviuvadan approved these changes

Assignees
No one assigned
Labels
semver:minor A change requiring a minor version bump type:documentation A documentation update
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@elliotgunton @samj1912 @flaviuvadan