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

CronWorkflow complains about missing CronWorkflowStatus attributes #636

Closed
2 of 4 tasks
iameskild opened this issue May 25, 2023 · 2 comments · Fixed by #637
Closed
2 of 4 tasks

CronWorkflow complains about missing CronWorkflowStatus attributes #636

iameskild opened this issue May 25, 2023 · 2 comments · Fixed by #637
Labels
type:bug A general bug

Comments

@iameskild
Copy link
Contributor

iameskild commented May 25, 2023

Pre-bug-report checklist

1. This bug can be reproduced using YAML

2. This bug occurs when...

  • running Hera code without submitting to Argo (e.g. when exporting to YAML)
  • running Hera code and submitting to Argo

Bug report

Describe the bug
When submitting a CronWorkflow to Argo, it complains that it is in violation of the Pydantic model. Upon closer inspection, it seems to be complaining specifically about the CronWorkflowStatus attributes (active, conditions, lastTimeScheduled). It was my understanding that these are not attributes that the user is responsible for setting. The CronWorkflow is still submitted and appears to run but the error message below is produced regardless.

Error log if applicable:

115, in create_cron_workflow
    w.create()
  File "/Users/eskild/miniconda3/envs/nebari/lib/python3.11/site-packages/hera/workflows/cron_workflow.py", line 82, in create
    return self.workflows_service.create_cron_workflow(
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/eskild/miniconda3/envs/nebari/lib/python3.11/site-packages/hera/workflows/service.py", line 395, in create_cron_workflow
    return CronWorkflow(**resp.json())
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "pydantic/main.py", line 342, in pydantic.main.BaseModel.__init__
pydantic.error_wrappers.ValidationError: 3 validation errors for CronWorkflow
status -> active
  none is not an allowed value (type=type_error.none.not_allowed)
status -> conditions
  none is not an allowed value (type=type_error.none.not_allowed)
status -> lastScheduledTime
  none is not an allowed value (type=type_error.none.not_allowed)

To Reproduce
Full Hera code to reproduce the bug:

from hera.workflows import CrongWorkflow, DAG, script

@script()
def foo(a, b=42, c=None):
    print(a, b, c)

with CronWorkflow(
    generate_name="cron-test-",
    entrypoint="D",
    schedule="* * * * *",
    starting_deadline_seconds=0,
    concurrency_policy="Replace",
    successful_jobs_history_limit=4,
    failed_jobs_history_limit=4,
    cron_suspend=False,
) as w:
    with DAG(name="D"):
        foo(name="b-unset-c-unset", arguments={"a": 1})
        foo(name="b-set-c-unset", arguments={"a": 1, "b": 2})
        foo(name="b-unset-c-set", arguments={"a": 1, "c": 2})
        foo(name="b-set-c-set", arguments={"a": 1, "b": 2, "c": 3})

w.create()

Expected behavior
A clear and concise description of what you expected to happen:

Environment

  • OS: [e.g. iOS] MacOS
  • Browser: [e.g. chrome, safari] Chrome
  • Version of Hera: [e.g. 5.1.6, 4.4.2] 5.2.0
  • Version of Argo: [e.g. 3.4.7] 3.4.4

Additional context
Add any other context about the problem here.

@flaviuvadan
Copy link
Collaborator

Thanks for the bug report @iameskild! I can reproduce the bug using your example. Let me know if #637 makes sense!

flaviuvadan added a commit that referenced this issue May 26, 2023
…eneration (#637)

<!-- Thank you for submitting a PR to Hera! 🚀 -->

**Pull Request Checklist**
- [x] Fixes #636 
- [ ] Tests added
- [x] Documentation/examples added
- [x] [Good commit messages](https://cbea.ms/git-commit/) and/or PR
title
<!-- Also remember to sign off commits or the DCO check will fail on
your PR! -->

**Description of PR**
<!-- If not linked to an issue, please describe your changes here -->

Currently, the Argo server does not return cron workflow statuses when a
users' cron workflow does not execute _immediately_ post submission. So,
`None`s are returned for the independent fields of the cron workflow
status. While the status is indeed optional, the fields are not! This PR
adds a special case for handling this in the service. I am not sure
whether this is a feature or a bug upstream in Argo Workflows.

CC: @iameskild I was able to run your example using the fix in this PR!

<!-- Piece of cake! ✨🍰✨ -->

---------

Signed-off-by: Flaviu Vadan <flaviuvadan@gmail.com>
Signed-off-by: Elliot Gunton <elliotgunton@gmail.com>
Co-authored-by: Elliot Gunton <egunton@bloomberg.net>
@iameskild
Copy link
Contributor Author

Thanks for the quick fix @flaviuvadan :)

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.

2 participants