Add Reaction for dapr output bindings #229
Conversation
Signed-off-by: nateb <[email protected]>
Signed-off-by: nateb <[email protected]>
Signed-off-by: nateb <[email protected]>
Signed-off-by: nateb <[email protected]>
Signed-off-by: nateb <[email protected]>
ms-nateb
force-pushed
the
dev/reaction-binding
branch
from
7c7c711 to
5932770
Compare
reactions/dapr/post-dapr-output-binding/Drasi.Reactions.PostDaprOutputBinding/QueryConfig.cs
Outdated
Show resolved
Hide resolved
| ?? throw new ArgumentNullException(nameof(config), $"Query configuration is null for query {queryId}"); | ||
|
|
||
| // Check if the query is already in a failed state | ||
| if (_failureTracker.IsQueryFailed(queryId)) |
There was a problem hiding this comment.
Polly is the most popular .net library for resilience - https://www.pollydocs.org/
I think we should look at adopting that instead of writing our own.
reactions/dapr/post-dapr-output-binding/Drasi.Reactions.PostDaprOutputBinding/Program.cs
Outdated
Show resolved
Hide resolved
| @@ -0,0 +1,59 @@ | |||
| .PHONY: all deploy-postgres deploy-redis deploy-dapr-components build-subscribers load-subscribers deploy-subscribers logs-alpha logs-beta clean-subscribers clean-dapr-components clean-redis clean-postgres clean-all | |||
There was a problem hiding this comment.
Can you please add an E2E tests like here?
# Conflicts: # .github/workflows/draft-release.yml # cli/installers/resources/default-reaction-providers.yaml
| "bindingName": "drasi-output-binding", | ||
| "bindingType": "http", | ||
| "bindingOperation": "put" | ||
| "packed": false, |
There was a problem hiding this comment.
Need to follow up on the enum instead of bool and add a comma
| name: PostDaprOutputBinding | ||
| spec: | ||
| services: | ||
| reaction: |
There was a problem hiding this comment.
We need to include the config schema section here. See the StorageQueue reaction for a relevant example.
reactions/dapr/post-output-binding/.idea/.idea.post-dapr-output-binding/.idea/indexLayout.xml
Outdated
Show resolved
Hide resolved
| /// <summary> | ||
| /// Implementation of the query failure tracker using ConcurrentDictionary. | ||
| /// </summary> | ||
| public class QueryFailureTracker : IQueryFailureTracker |
There was a problem hiding this comment.
Could we use a library like Polly for this?
https://github.com/App-vNext/Polly
There was a problem hiding this comment.
Also, does invoking through Dapr not already add circuit breaker functionality?
There was a problem hiding this comment.
Dapr provides a default circuit breaker https://docs.dapr.io/operations/resiliency/policies/default-policies/ would we need to extend it?
There was a problem hiding this comment.
We would not need to do anything. If Dapr is already providing this functionality, we don't need to build it into the Reaction.
…t-binding/.idea directory
…n.DotSettings.user
Waiting on #226 will update with changes made and then rebase!
This pull request introduces a new reaction,
PostDaprOutputBindings, to the system. It includes updates to configuration files, Dockerfiles, and the addition of unit tests for the new functionality. Below is a summary of the key changes:Dockerfile Additions
DockerfileforPostDaprOutputBindingto build and publish the reaction using .NET 9.0, with production-specific logging configurations.Dockerfile.debugfor debugging purposes, including additional runtime dependencies and debug-specific logging configurations.Unit Tests for New Reaction
ChangeFormatterinDrasi.Reactions.PostDaprOutputBinding.Tests/ChangeFormatterTests.cs, covering scenarios like formatting added, updated, and deleted results.ChangeHandlerinDrasi.Reactions.PostDaprOutputBinding.Tests/ChangeHandlerTests.cs, testing behavior for various configurations, error handling, and event publishing.# DescriptionPlease explain the changes you've made.
Type of change
Implements #248