Fix/221 retrypolicy

[CasperGN]

Description

Implement default retry policy and allow overwriting during agent instantiation.

Issue reference

We strive to have all PR being opened based on an issue, where the problem or feature have been discussed prior to implementation.

Please reference the issue this PR closes: #221

Checklist

Please make sure you've completed the relevant tasks for this PR, out of the following list:

• Created/updated tests
• Tested this change against all the quickstarts
• Extended the documentation
  • Created the dapr/docs PR: docs: document durable agent retry policy docs#4982

Note: We expect contributors to open a corresponding documentation PR in the dapr/docs repository. As the implementer, you are the best person to document your work! Implementation PRs will not be merged until the documentation PR is opened and ready for review.

[bibryam]

Casper, thank you for working on this feature. I love this feature!

I think we should make sure that these attributes that are all related to each other in the context of a retry should be somehow grouped together.

I was thinking that we could add this within the agent execution config, such as this

 execution=AgentExecutionConfig(
            max_iterations=5
            retry_max_attempts=10,
            retry_initial_backoff=5,
            retry_max_backoff=45,
            retry_backoff_multiplier=1.5,
        ),

but this does not apply to regular agents. As a result, we cannot include it there.

This is the best I can come up with.

        retry=DurableRetryConfig(
            max_attempts=10,
            initial_backoff=5,
            max_backoff=45,
            backoff_multiplier=1.5,
        ),

This has few benefits:

1. It shows the fields that are related to each other.
2. They are about retry behaviour
3. It makes clear that this is a durable retry different than the in-memory retries,
4. This config can be set to durable agents only

HTH

[CasperGN]

@bibryam have a look at the updates

[sicoyle]

few comments so far 🙏

[sicoyle]

I think this should be more workflow-specific. Something like WorkflowRetryConfig might be clearer, since this behavior only applies to the durable agent anyway, which makes “durable” feel a bit redundant here.

On retries more broadly, should we consider introducing a resiliency policy in our quickstarts—or at least documenting it—so users know how to apply retry policies for state operations and pub/sub? For example, we could reference the existing Dapr resiliency quickstart or general docs on this:
https://docs.dapr.io/getting-started/quickstarts/resiliency/resiliency-state-quickstart/

[CasperGN]

I'll relabel this WorkflowRetryPolicy to be explicit on it

[sicoyle]

any reason there are four fields here yet docs show 5 options we can set?
https://docs.dapr.io/developing-applications/building-blocks/workflow/workflow-features-concepts/#retry-policies

[CasperGN]

It's an oversight on my part. I've added the 5th as well!

[sicoyle]

we should consider if we want to deviate here. The default that i see in docs is actually a 0 value for this... which I think for durable agent durability we want retries enabled, just want confirmation on that :)
https://docs.dapr.io/developing-applications/sdks/python/python-client/#initialising-the-client

[CasperGN]

I have to check on 1, else it might fail upstream

[cicoyle]

The issue is asking for infinite retries by default. In dapr we have -1 in places to signal infinity. Would it make sense to allow for infinite retries here in that same manner as well?

[CasperGN]

Unfortunately that's not how it is in the upstream sdk

[cicoyle]

so as-is, there is no way to specify retries indefinitely? maybe we should tweak the upstream sdk eventually to support this :)

this PR is a good start!

[CasperGN]

@cicoyle yes I concur. This should be fixed upstream in the python-sdk then carried down here. I'll put it on our todo list!

[CasperGN]

@sicoyle, @cicoyle I've added dapr/python-sdk#870 for upstream fixing

[sicoyle]

LGTM - thank you!!