Fix validation error for retry strategy

[lakhanjindam]

Fix issue #552

[flaviuvadan]

Thanks for submitting the PR @lakhanjindam! 🚀

There might be something strange happening here.. there's a validator for this field that is supposed to convert the limit from str | int to IntOrString

[lakhanjindam]

problems seems to be happening with this line https://github.com/argoproj-labs/hera/blob/main/src/hera/workflows/retry_strategy.py#L55
since i made change where limit is mapped to IntOrString type so it goes into if block and returns from there.

[flaviuvadan]

Ah, yes, you are right... I think, though, that we should enrich this type so that users can pass in 1 of 3 types:

Suggested change

	limit: Optional[IntOrString] = None
	limit: Optional[Union[int, str, IntOrString]] = None

Also, you pointed out the __root__ usage issue. If we change line 55 from

return IntOrString(__root__=str(v)) # int or str

to

return str(v)  # int or str

everything should work.

[lakhanjindam]

yeah this will work. But what if the value passed is bool for var limit, in that case it wont raise any err since it will be converting it to str type

[lakhanjindam]

So this is happening, i did some simple debugging on the value we are returning.
When we enter the validator function for limit var

def _convert_limit(cls, v):
    if v is None or isinstance(v, IntOrString):
        return v
    return IntOrString(__root__=str(v))  # __root__='3'
   # ------- fix ---------
   v = IntOrString(__root__=str(v))
   return v.__root__

So the value which is returned is not a valid type logically, hence its throwing error.
ref: https://docs.pydantic.dev/usage/models/#custom-root-types

[flaviuvadan]

I left a comment on an implementation that I think will solve the problem! Also, any chance you can please add an upstream example in examples/workflows/upstream/retry_script.py? This will map to the following example: https://github.com/argoproj/argo-workflows/blob/master/examples/retry-script.yaml

Note that you will have to run make regenerate-test-data to generate the YAML for comparison/testing!

from hera.workflows import Workflow, script, RetryStrategy


@script(image="python:alpine3.6", retry_strategy=RetryStrategy(limit=10), add_cwd_to_sys_path=False)
def retry_script():
    import random
    import sys
    exit_code = random.choice([0, 1, 1])
    sys.exit(exit_code)


with Workflow(generate_name="retry-script-", entrypoint="retry-script") as w:
    retry_script()

[flaviuvadan]

Thank you for the fix @lakhanjindam!

Might need a quick make format before we can merge

[flaviuvadan]

@lakhanjindam the PR looks great to me so I am merging it into an upstream branch that I will run make format for you on then merge that one! Thank you again 🚀