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

Split up script user guide and fix pydantic io example #982

Merged
merged 3 commits into from
Mar 4, 2024

Conversation

elliotgunton
Copy link
Collaborator

  • Script user guide was too long, split into main features. Fix internal links
  • Make pydantic io example into a runnable workflow - made it more obvious the scripts would need a custom image

Pull Request Checklist

Description of PR
Currently, the pydantic IO example does not work. Combined with fixes in PRs #974 and #977, this doc change shows how to use the Runner IO features.

* Script user guide was too long, split into main features. Fix internal links
* Make pydantic io example into a runnable workflow - made it more obvious the scripts
would need a custom image

Signed-off-by: Elliot Gunton <egunton@bloomberg.net>
Signed-off-by: Elliot Gunton <egunton@bloomberg.net>
```python
@script()
def echo_all(
an_int: Annotated[int, Parameter(description="an_int parameter", default=1, enum=[1, 2, 3])],
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For future dev - would it be more Python to support setting the default field kwarg style?

Suggested change
an_int: Annotated[int, Parameter(description="an_int parameter", default=1, enum=[1, 2, 3])],
an_int: Annotated[int, Parameter(description="an_int parameter", enum=[1, 2, 3])] = 1,

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point, will change here but we have #812 to remove the non-kwarg syntax

Copy link
Collaborator

@flaviuvadan flaviuvadan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎖️ this is huge and it looks awesome

Signed-off-by: Elliot Gunton <egunton@bloomberg.net>
@elliotgunton elliotgunton merged commit 18902f7 into runner-refactor Mar 4, 2024
3 of 5 checks passed
@elliotgunton elliotgunton deleted the runner-docs-update branch March 4, 2024 16:55
elliotgunton added a commit that referenced this pull request Mar 4, 2024
**Pull Request Checklist**
- [x] Fixes #962 
- [x] Tests added
- [ ] Documentation/examples added
- [x] [Good commit messages](https://cbea.ms/git-commit/) and/or PR
title

**Description of PR**
Currently, defaults are required when using Parameters in the
RunnerInput. This PR allows defaults to be omitted.

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

**Description of PR**
Primarily, this PR refactors the runner code. It also fixes the mapping and loading of kwargs to a RunnerInput object.

Currently, the runner code is hard to follow. This PR refactors the functionality in the `runner.py` file into a `_runner` module with util submodules. The PR also makes the complex input mapping logic more testable.

## Changes from #982 

* Script user guide was too long, split into main features. Fix internal links
* Make pydantic io example into a runnable workflow - made it more obvious the scripts would need a custom image

**Pull Request Checklist**
- [x] Fixes #961 (fixes example)
- [ ] Tests added
- [x] Documentation/examples added
- [x] [Good commit messages](https://cbea.ms/git-commit/) and/or PR title

**Description of PR**
Currently, the pydantic IO example does not work. Combined with fixes in PRs #974 and #977, this doc change shows how to use the Runner IO features.

---------

Signed-off-by: Elliot Gunton <egunton@bloomberg.net>
elliotgunton added a commit that referenced this pull request Mar 4, 2024
Fix doc links broken in #982

Signed-off-by: Elliot Gunton <egunton@bloomberg.net>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
semver:patch A change requiring a patch version bump type:documentation A documentation update
Projects
None yet
Development

Successfully merging this pull request may close these issues.

RunnerInput: Got 400 from Argo, a value was not supplied in the parameter
2 participants