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

Add timer sample #57

Merged
merged 6 commits into from
Apr 2, 2024
Merged

Add timer sample #57

merged 6 commits into from
Apr 2, 2024

Conversation

lorensr
Copy link
Contributor

@lorensr lorensr commented Mar 28, 2024

Thinking maybe we should add handling workflow cancellation to this sample. Wdyt, and would this be the right way?

namespace TemporalioSamples.Timer;

using Microsoft.Extensions.Logging;
using Temporalio.Workflows;
using System.Threading.Tasks;

[Workflow]
public class MyWorkflow
{
    [WorkflowRun]
    public async Task RunAsync()
    {
        try
        {
            while (true)
            {
                Workflow.CancellationToken.ThrowIfCancellationRequested();

                await Workflow.DelayAsync(1000, Workflow.CancellationToken);

                var result = await Workflow.ExecuteActivityAsync(
                    () => MyActivities.Charge(user),
                    new() { StartToCloseTimeout = TimeSpan.FromMinutes(5) },
                    Workflow.CancellationToken); // Pass the cancellation token

                Workflow.Logger.LogInformation("Activity result: {Result}", result);
            }
        }
        catch (OperationCanceledException)
        {
            // Handle any cleanup upon cancellation here
            Workflow.Logger.LogInformation("Workflow cancelled.");
        }
    }
}

Copy link
Member

@cretz cretz left a comment

Choose a reason for hiding this comment

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

Thinking maybe we should add handling workflow cancellation to this sample. Wdyt, and would this be the right way?

The workflow cancellation token is already the default cancellation token for these calls if none provided so you don't have to change any workflow code.

src/Timer/Subscription.workflow.cs Outdated Show resolved Hide resolved
SetMinimumLevel(LogLevel.Information)),
});

async Task RunWorkerAsync()
Copy link
Member

Choose a reason for hiding this comment

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

We haven't on most samples, but we may find using https://github.com/temporalio/sdk-dotnet/tree/main/src/Temporalio.Extensions.Hosting for worker running to be easier in our samples here and elsewhere (this is what many of our users use)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Making worker file shorter sgtm, going to leave this for future work for time's sake.

@lorensr lorensr requested a review from cretz March 28, 2024 23:20
Copy link
Member

@cretz cretz left a comment

Choose a reason for hiding this comment

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

OK, can merge. I have opened #59 for finishing out the sample.

@lorensr lorensr merged commit 61133fb into main Apr 2, 2024
6 checks passed
@cretz cretz deleted the timer branch April 2, 2024 22:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants