Allow IGrainWithGuidCompoundKey as implicit subscription grain #3011
Conversation
src/Orleans/Core/GrainExtensions.cs
Outdated
| { | ||
| var key = grain.GetPrimaryKey(out string keyExtension); | ||
| return new StreamIdentity(key, keyExtension); | ||
| } |
There was a problem hiding this comment.
GrainExtention is not a good place to put this method, since this method is not commonly applicable to all grains. This method only applies to grains which subscribes to a stream implicitly.
| { | ||
| logger.Info($"************************ {nameof(FilteredImplicitSubscriptionWithExtensionGrainTest)} *********************************"); | ||
|
|
||
| var streamNamespaces = new[] { "red1", "red2", "blue3", "blue4" }; |
There was a problem hiding this comment.
why do you need this variable? it is not used in anywhere
src/Orleans/Core/GrainExtensions.cs
Outdated
|
|
||
| public static StreamIdentity GetImplicitStreamIdentity(this IGrainWithGuidCompoundKey grain) | ||
| { | ||
| var key = grain.GetPrimaryKey(out string keyExtension); |
There was a problem hiding this comment.
Compilation fails due to the use of this C# 7 feature here.
|
@dotnet-bot test netfx-functional please |
|
LG2M |
|
Breaking change note: GrainId for implicit subscribed consumer grain which implements IGrainWithGuidCompoundKey will change from stream.GuidId to stream.NameSpace + stream.GuidId. A workaround if you don't want this feature : make the consumer grain implements IGrainWithGuidKey breaking change note is here for better tracking breaking change. |
| @@ -0,0 +1,12 @@ | |||
| namespace Orleans.Streams | |||
| { | |||
| public static class ImplicitConsumerGrainExtensions | |||
There was a problem hiding this comment.
do you need to add xml comments here? build your solution and fix warnings if any,
There was a problem hiding this comment.
@xiazen It wasn't giving warnings but I've added xml docs anyway.
|
Thanks @DixonDs ! will merge it after the test finished |
|
@dotnet-bot test netfx-functional please |
Per discussion in #2987