Dotnet Update Features

[Sushisource]

What was changed

Added update features for .net

Why?

Checklist

1. Closes [Feature Request] Update feature tests #266

2. How was this tested:

3. Any docs updates needed?

[cretz]

Only minor things. Overally I'm happy to see how legible the C# logic and workflows are.

[cretz]

Suggested change

	using Temporalio.Exceptions;
	namespace update.basic_async;
	namespace update.basic_async;
	using Temporalio.Exceptions;

Namespace first usually

[Sushisource]

yeah, stupid IDE kept doing this

[cretz]

I wonder if there would be value in a single RestartWorker call.

[Sushisource]

Maybe, but, it's a bit less flexible

[cretz]

Might as well move this duplicated exception swallower to inside StopAndWait

[cretz]

Suggested change

	workerTask = worker.ExecuteAsync(workerStopper.Token);
	workerTask = Task.Run(
	async () =>
	{
	try
	{
	await worker.ExecuteAsync(workerStopper.Token);
	}
	catch (Exception e)
	{
	// Log e
	throw;
	}
	});

The existing code is benefitted by the way we wrote the internals of this call that it's not an async, but by default in .NET tasks don't start just because you invoke a method that returns a task, so Task.Run is best.

Also, technically the problem with the split start/stop approach (and why we avoided it in the .NET worker API) is because you can't see fatal worker exceptions. So that's what the catch here is for. But if we aren't concerned with that, no prob. But you might as well swallow the cancelled exception here.

[Sushisource]

It's not a matter of not being concerned with it - it's that if I need to stop a worker and start a new one at any time, the RunUntil approach just doesn't really fit.

In fact it would be really nice to have a way to stop the worker that doesn't force an exception to be thrown. That feels like a bit of an artificial burden.

[Sushisource]

This seems to work jut fine without Task.Run too - I thought .NET always eagerly Ran tasks? In any case I put it inside Task.Run but w/o the try/catch since that would log every time it's cancelled and would need the special-case for that too, and the task gets awaited on anyway, so any thrown exception will bubble up.

[cretz]

In fact it would be really nice to have a way to stop the worker that doesn't force an exception to be thrown. That feels like a bit of an artificial burden.

You can by having something like a task completion source and waiting on it in the lambda passed to worker execute. It's just that normal .NET code that relies on cancellation token is expected to throw that exception, that's why I had to.

I thought .NET always eagerly Ran tasks?

No, unfortunately not (same with Python), but if you write a method that returns a Task instead of an async method that returns a task, that runs eagerly. That's why this worked before, because that's what we did internally.

but w/o the try/catch since that would log every time it's cancelled and would need the special-case for that too, and the task gets awaited on anyway, so any thrown exception will bubble up.

But the exception isn't bubbled up anywhere until StopAndWait. That's why I mentioned logging, just in case of fatal worker failure before we ask it to stop. But I am fine with that if you are.

[cretz]

Suggested change

	workerTask = Task.Run(async () => await worker.ExecuteAsync(workerStopper.Token));
	workerTask = Task.Run(() => worker.ExecuteAsync(workerStopper.Token));

I think this should also work, but it's basically the same thing so no need to change.