feat: Add NotifyOrchestrationInstance trigger to Api #102
Conversation
ebbeknudsen
had a problem deploying
to
AzureAuth
GitHub Actions
Failure
ebbeknudsen
had a problem deploying
to
AzureAuth
GitHub Actions
Failure
ebbeknudsen
had a problem deploying
to
AzureAuth
GitHub Actions
Failure
ebbeknudsen
had a problem deploying
to
AzureAuth
GitHub Actions
Failure
ebbeknudsen
had a problem deploying
to
AzureAuth
GitHub Actions
Failure
ebbeknudsen
had a problem deploying
to
AzureAuth
GitHub Actions
Failure
ebbeknudsen
had a problem deploying
to
AzureAuth
GitHub Actions
Failure
ebbeknudsen
had a problem deploying
to
AzureAuth
GitHub Actions
Failure
ebbeknudsen
had a problem deploying
to
AzureAuth
GitHub Actions
Failure
| /// <summary> | ||
| /// Start a BRS-026 request. | ||
| /// </summary> |
There was a problem hiding this comment.
I believe this comment can be removed?
| nameof(majorVersion), | ||
| majorVersion, | ||
| $"Unhandled major version in received notify service bus message (MessageId={message.MessageId})"), | ||
| }; |
There was a problem hiding this comment.
Personally I prefer the indention to only be 4 chars
| { | ||
| Data = | ||
| { | ||
| { nameof(message.MessageId), message.MessageId }, | ||
| { nameof(message.Subject), message.Subject }, | ||
| { "MajorVersion", message.TryGetMajorVersion() }, | ||
| { "TargetType", typeof(TMessage).Name }, | ||
| }, | ||
| }, |
There was a problem hiding this comment.
If this belongs to the exception, it needs to be indented 4 chars I believe
| public enum ServiceBusMessageBodyFormat | ||
| { | ||
| Binary = 1, | ||
| Json = 2, | ||
| } |
There was a problem hiding this comment.
Could we move this to a separate file?
| public static TMessage ParseMessageBody<TMessage>(this ServiceBusReceivedMessage message) | ||
| where TMessage : IMessage<TMessage>, new() | ||
| { | ||
| var bodyFormat = message.GetBodyFormat(); |
There was a problem hiding this comment.
There is a lot of validation in various methods, so its a bit difficult to parse if it actually makes sense. But I believe that the GetBodyFormat will throw an exception if the format is unexpected, so I don't think the switch on bodyFormat can ever throw the ArgumentOutOfRangeException.
There was a problem hiding this comment.
That is correct, however switch expressions requires a default (_) arm.
| @@ -32,4 +32,8 @@ | |||
| <CopyToPublishDirectory>Never</CopyToPublishDirectory> | |||
| </None> | |||
| </ItemGroup> | |||
|
|
|||
There was a problem hiding this comment.
Maybe remove this from current PR
|
|
||
| <ItemGroup> |
There was a problem hiding this comment.
Maybe remove this from current PR
There was a problem hiding this comment.
This filename is incorrect or at least not matching the class name.
Should this perhaps be moved to another PR?
| try | ||
| { | ||
| context.SetCustomStatus(CustomStatus.WaitingForEnqueueActorMessages); | ||
| await context.WaitForExternalEvent<int?>( |
There was a problem hiding this comment.
No need for <int?>, or am I missing somthing?
There was a problem hiding this comment.
The method requires a type parameter, I couldn't find any without one.
| await context.WaitForExternalEvent<int?>( | ||
| eventName: RequestCalculatedWholesaleServicesNotifyEventsV1.EnqueueActorMessagesCompleted, |
There was a problem hiding this comment.
The method requires a type parameter, I couldn't find any without one.
Description
NotifyOrchestrationInstanceTriggerINotifyOrchestrationInstanceCommandsand implementNotifyOrchestrationInstanceAsyncReferences
Link to assignment:
Checklist