-
Notifications
You must be signed in to change notification settings - Fork 36
Add RetryPolicy to ActivityHeartbeatingCancellation sample #121
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
Conversation
…mple by demonstrating how Activity heartbeats interact with RetryPolicy configured at the workflow level.
|
|
| ## How to Run | ||
|
|
||
| 1. Start Temporal locally (`temporalite start`). | ||
| 2. Run the worker: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems to be incomplete and we don't use "temporalite" (other samples say to look at primary README for prerequisites to setup server)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, agreed. I’ll align this README with other samples by removing the temporalite mention and pointing to the primary README prerequisites for running a Temporal server.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder if this really deserves to be a sample though. It's simply showing how to change two defaults for a RetryPolicy property, correct? Not sure it relates to heartbeat/cancellation or that it deserves a sample to show how to set retry policy settings.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the feedback to all comments. Makes sense. I understand the desire to keep the sample narrowly focused.
I’ll close this PR. Appreciate the review and discussion.
| in Temporal's .NET SDK. | ||
|
|
||
| ## What This Sample Shows | ||
| - How to configure retry settings for an activity. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Heartbeating sample seems unrelated to retry settings that you may want to set whether or not you heartbeat. Is this addition to the sample only for showing that you can set the RetryOptions property? This can be set regardless. I am not sure it makes sense in a heartbeating/cancellation sample to demonstrate that this setting can be set (any more than any other activity option setting).
| MaximumAttempts = 3, | ||
| InitialInterval = TimeSpan.FromSeconds(1), | ||
| BackoffCoefficient = 2.0, | ||
| MaximumInterval = TimeSpan.FromSeconds(10) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Two of these are the defaults already, and the other two are just choices a user may make regardless of heartbeating/cancellation.
What was changed
This PR enhances the existing ActivityHeartbeatingCancellation sample by
demonstrating how Activity heartbeats can be combined with RetryPolicy
configured at the workflow level.
Why?
This helps .NET developers understand best practices for long-running,
resumable activities.
Checklist
Closes
How was this tested:
Documentation change
Any docs updates needed?