Add dead letter topic support#929
Conversation
Codecov Report
@@ Coverage Diff @@
## master #929 +/- ##
==========================================
- Coverage 69.87% 69.70% -0.18%
==========================================
Files 154 155 +1
Lines 5079 5119 +40
Branches 549 553 +4
==========================================
+ Hits 3549 3568 +19
- Misses 1399 1420 +21
Partials 131 131
Flags with carried forward coverage won't be shown. Click here to find out more.
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
There was a problem hiding this comment.
Is there a reason we aren't using the debugger? Also, this line implies that we're getting the deposit from StdIn or something, but I don't think that's the case?
There was a problem hiding this comment.
Are these duplicates to the attributes?
There was a problem hiding this comment.
Just realized this was in the RoutingSample and not the ControllerSample. Nevermind.
There was a problem hiding this comment.
Were there no changes needed for the program that runs the example? Or updates to the README that explains it?
There was a problem hiding this comment.
I'll probably add a small section to test the dead letter topic by providing a negative amount to be deposited/withdrawn
There was a problem hiding this comment.
We're adding the attribute as metadata to the builder?
There was a problem hiding this comment.
I just copied the concept from the previous method: https://github.com/dapr/dotnet-sdk/blob/master/src/Dapr.AspNetCore/DaprEndpointConventionBuilderExtensions.cs#L96
There was a problem hiding this comment.
You have some extra space at the end of this line.
There was a problem hiding this comment.
Just to check, this is copied from dapr/dapr, right?
There was a problem hiding this comment.
This change isn't required unless you want to use it in your test.
There was a problem hiding this comment.
I had to add it because I had added DeadLetterTopic to ITopicMetadata. Fixed since I added a new interface for dead letter topic metadata.
halspang
left a comment
halspang
left a comment
There was a problem hiding this comment.
Just some minor feedback and you're good to go.
There was a problem hiding this comment.
We should still be using the logger.
There was a problem hiding this comment.
Nit, first comment should end in a ..
Closes dapr#897 Signed-off-by: Yash Nisar <yashnisar@microsoft.com>
2 participants
Closes #897
Signed-off-by: Yash Nisar yashnisar@microsoft.com
Description
Add support for dead letter topic for the dotnet sdk
Issue reference
We strive to have all PR being opened based on an issue, where the problem or feature have been discussed prior to implementation.
Please reference the issue this PR will close: #897
Checklist
Please make sure you've completed the relevant tasks for this PR, out of the following list: