Removes the IGrainWithGuidKey constraint from ISyncWorker.

[Kritner]

Description

This is a breaking change to the package, but does allow for more control over the implementations of long running grains.

Removes the IGrainWithGuidKey constraint from ISyncWorker.

ISyncWorker was defined as:

public ISyncWorker<in TRequest, TResult> : IGrainWithGuidKey

Now it is defined as:

public ISyncWorker<in TRequest, TResult> : IGrain

This will allow implementations against the base class SyncWorker<TRequest, TResult> to specify the key type that fits their needs.

Previously, a "grain interface" was not required for implementations, now it will certainly make things easier to grok, albeit with an additional interface per long running grain.

Old implementation:

public class PasswordVerifier : SyncWorker<PasswordVerifierRequest, PasswordVerifierResult>
{
  // ...
}

New implementation:

// now specifying the key type in the grain interface
public interface IPasswordVerifierGrain : ISyncWorker<PasswordVerifierRequest, PasswordVerifierResult>, IGrainWithGuidKey

public class PasswordVerifierGrain : SyncWorker<PasswordVerifierRequest, PasswordVerifierResult>, IPasswordVerifierGrain
{
  // ...
}

Old calling code:

var grain = grainFactory.GetGrain<ISyncWorker<PasswordVerifierRequest, PasswordVerifierResult>>(Guid.NewGuid());

New calling code:

var grain = grainFactory.GetGrain<IPasswordVerifierGrain>(Guid.NewGuid());

Fixes #52

Type of Change

Use an x in between the [ ] for each line applicable to the type of change for this PR

• Bug fix
• New Feature
• Documentation
• Code improvement
• Breaking change - if a public API changes, or any change that DOES or MAY require a major revision to the NuGet package version due to semver.
• Unit tests
• Code samples
• Added your repository URL to the readme because you make use of this super cool package! ;)
• Other

Describe testing that was performed for your change

Updated existing tests

Checklist

• Read the https://github.com/OrleansContrib/Orleans.SyncWork/blob/main/CONTRIBUTING.md
• Ran unit tests and ensured they passed
• Added unit tests where applicable
• Referenced an issue where applicable
• Ran dotnet-format locally

[Kritner]

@hendrikdevloed this about what you expected?

[hendrikdevloed]

Wow that was quick, I'll check today and tomorrow if I can fit this in my program's grain model and give feedback. Thanks!

[Kritner]

@hendrikdevloed you good compiling from this branch? Or do i need to figure out how to get a pre-release package deployed?

[hendrikdevloed]

Thanks, I'm fine, I took the branch's source into my project and temporarily removed the NuGet reference.

[hendrikdevloed]

A tangential question while I was deriving my grain from SyncWorker: is there a reasoning behind throwing an exception from GetWorkStatus() when the grain has not been started yet, versus returning NotStarted?

if (_status == SyncWorkStatus.NotStarted)
{
    Logger.LogError("{Method} was in a status of {WorkStatus}", nameof(GetWorkStatus), SyncWorkStatus.NotStarted);
    DeactivateOnIdle();
    throw new InvalidStateException(_status);
}
return Task.FromResult(_status);

I ask because I intended to start a long-running task (in my project a sequenceFactoryGrain eventually creates a sequenceGrain, working as a long-running task) in an idempotent way, like

ISequenceGrain sequenceGrain = await sequenceFactoryGrain.GetWorkStatus() switch
{
    SyncWorkStatus.NotStarted => await sequenceFactoryGrain.StartWorkAndPollUntilResult(sequence),
    SyncWorkStatus.Running => await sequenceFactoryGrain.ContinueWorkAndPollUntilResult(sequence),
    SyncWorkStatus.Completed => (await sequenceFactoryGrain.GetResult())!,
    SyncWorkStatus.Faulted => throw (await sequenceFactoryGrain.GetException())!,
    _ => throw new NotImplementedException()
};

but was surprised by the exception thrown when trying to check the current state.

My intent: If some external non-Orleans API call tries to start the factory to get a grain, but if somehow there is a timeout/cancellation/... and the API retries, I should be able to find out that the sequenceFactoryGrain is already processing and just tag along waiting for the long-running call to complete.
(ContinueWorkAndPollUntilResult is the same code as StartWorkAndPollUntilResult but without the Start call)

PS: do you still actively hang out on the Orleans Discord? Otherwise we could take this off-topic thread over there?

[Kritner]

is there a reasoning behind throwing an exception from GetWorkStatus() when the grain has not been started yet, versus returning NotStarted?

No reasoning - and I'd have no qualms w/ taking that check out

If you want to open an issue about changing the behavior, I could get that rolled into the next nuget package release along w/ #53 and #55

PS: do you still actively hang out on the Orleans Discord? Otherwise we could take this off-topic thread over there?

I don't really take a look at it much these days, got a bit too noisy, and I'm not actively working w/ Orleans atm.

[Kritner]

Just since we had some additional back and forth here - lmk if/when you've evaluated the change and if it's g2g

[Kritner]

Yo @hendrikdevloed just wanted to check in to see if you've had a chance to try this out? Was hoping to get this merged in, cuz it's going to have some conflicts with #53

[hendrikdevloed]

Hi Russ,

I've managed to run it with a string based key, so I assume it is working. I'm refactoring a part of my application and can't test my new design that uses the different grain key type thoroughly, so I'm not able to confirm with 100% certainty yet.

But if you want to merge it, you definitely have my blessing, it looks fine.

Thanks for the swift fix,
Hendrik