Review debug reaction #155

ruokun-niu · 2025-02-19T01:09:39Z

Description

Use React instead of Blazor to build the frontend
Updated the code to use the Reaction SDK
Updated the raw event stream to update automatically when a new event is received
Updated the reaction provider with frontend and backend pod configs

Type of change

This pull request is a minor refactor, code cleanup, test improvement, or other maintenance task and doesn't change the functionality of Drasi (issue link optional).

Fixes: #issue_number

This reverts commit 5f3d793.

reactions/platform/debug-reaction/debug-reaction.Server/Properties/launchSettings.json

reactions/platform/debug-reaction/debug-reaction.client/src/pages/QueryPage.jsx

reactions/platform/debug-reaction/debug-reaction.client/vite.config.js

reactions/platform/debug-reaction/debug-reaction.Server/Services/ResultViewClient.cs

reactions/platform/debug-reaction/debug-reaction.client/src/App.jsx

reactions/platform/debug-reaction/debug-reaction.client/src/main.jsx

reactions/platform/debug-reaction/debug-reaction.client/src/pages/EventStream.jsx

reactions/platform/debug-reaction/debug-reaction.client/src/pages/QueryPage.jsx

reactions/sdk/dotnet/Drasi.Reaction.SDK/Reaction.cs

danielgerlag · 2025-02-19T18:50:08Z

reactions/platform/debug-reaction/debug-reaction.Server/Controllers/QueryController.cs

+
+[ApiController]
+[Route("query")]
+public class QueryController : ControllerBase


We should follow RESTful conventions. Having a queries and a query route is confusing.
eg.
GET / PUT / DELETE /query/<id>

Planning to implement the followings:

queries

used for retrieving a list of queries that the reaction is subscribed to

queries/{queryId}

for retrieving the current result set of a query. This is used when the page loads up and when the user clicks the reinitialize query cache button

queries/{queryId}/debug-information

for debug info like query host, status and etc.

reactions/platform/debug-reaction/debug-reaction.Server/Models/QueryResult.cs

reactions/platform/debug-reaction/debug-reaction.Server/Services/IQueryDebugService.cs

reactions/platform/debug-reaction/debug-reaction.Server/Services/QueryDebugService.cs

reactions/platform/debug-reaction/debug-reaction.Server/Controllers/QueryController.cs

reactions/platform/debug-reaction/debug-reaction.Server/Services/ResultViewClient.cs

reactions/platform/debug-reaction/debug-reaction.Server/Program.cs

reactions/platform/debug-reaction/debug-reaction.client/src/pages/QueryPage.jsx

reactions/platform/debug-reaction/debug-reaction.Server/Program.cs

reactions/platform/debug-reaction/debug-reaction.Server/Services/QueryDebugService.cs

danielgerlag · 2025-02-28T01:11:10Z

reactions/platform/debug-reaction/debug-reaction.Server/Services/QueryDebugService.cs

+		}
+
+
+		private async Task<QueryResult> InitResult(string queryId)


What happens if this method is invoked for the same query twice, before the first one completes?

Good question. I did some searches online and it seems like one promising approach is to store/cache Task using a ConcurrentDicitionary. That way concurrent invocation to this method will share the same result. Any thoughts on this approach @danielgerlag ?

reactions/platform/debug-reaction/debug-reaction.Server/Services/QueryDebugService.cs

danielgerlag · 2025-02-28T01:24:18Z

reactions/platform/debug-reaction/debug-reaction.client/src/pages/EventStream.jsx

+
+    const fetchStream = async () => {
+        try {
+            const url = "http://localhost:5195/stream";


If the front end and back end run in separate pods, how would localhost reach the backend?

Currently we are still relying on port-forwarding both the frontend and the backend pods. I haven't looked into intra-cluster communication using k8s services yet, but I'll spend some time during this iteration

You could serve the front-end app content from the / route of the backend, and it could connect to the /api/* routes for the same address. That way, we don't need two containers.

Let me give it a try

Was able to get it working! Just need to clean up a few things

reactions/platform/debug-reaction/Program.cs

+
+
+		var port = Environment.GetEnvironmentVariable("PORT") ?? "8080";
+		server.Urls.Add($"http://0.0.0.0:{port}");


reactions/platform/debug-reaction/Properties/launchSettings.json

-  }
-}
+{
+  "$schema": "http://json.schemastore.org/launchsettings.json",


reactions/platform/debug-reaction/Properties/launchSettings.json

+    "windowsAuthentication": false,
+    "anonymousAuthentication": true,
+    "iisExpress": {
+      "applicationUrl": "http://localhost:30442",


reactions/platform/debug-reaction/Properties/launchSettings.json

+      "dotnetRunMessages": true,
+      "launchBrowser": true,
+      "launchUrl": "swagger",
+      "applicationUrl": "https://localhost:7230;http://localhost:5195",


reactions/platform/debug-reaction/Properties/launchSettings.json

+      "dotnetRunMessages": true,
+      "launchBrowser": true,
+      "launchUrl": "swagger",
+      "applicationUrl": "https://localhost:7230;http://localhost:5195",


…/drasi-platform into review-debug-reaction * 'review-debug-reaction' of https://github.com/ruokun-niu/drasi-platform: Transition to Native Multi-Platform Builds for Docker Images (drasi-project#161) Bump jinja2 from 3.1.4 to 3.1.5 in /reactions/sdk/python (drasi-project#135) Bump vite in /reactions/signalr/signalr-reaction/clients/vue (drasi-project#142) Bump nanoid in /reactions/signalr/signalr-reaction/clients/react/example (drasi-project#131) Bump openssl from 0.10.66 to 0.10.71 in /query-container (drasi-project#160) Bump golang.org/x/crypto from 0.26.0 to 0.31.0 in /cli (drasi-project#132) Bump nanoid in /reactions/signalr/signalr-reaction/clients/vue/example (drasi-project#130) Bump openssl from 0.10.68 to 0.10.71 in /sources/sdk/rust/example/proxy (drasi-project#158) Bump io.undertow:undertow-core from 2.3.15.Final to 2.3.17.Final in /sources/sdk/java (drasi-project#127) Bump esbuild from 0.16.17 to 0.25.0 in /dev-tools/vscode/drasi (drasi-project#151) Bump openssl from 0.10.66 to 0.10.70 in /control-planes/mgmt_api (drasi-project#148) Remove Fixed Dapr Version Constraint (drasi-project#147) Gremlin Reaction hotfix (drasi-project#149)

danielgerlag · 2025-03-10T17:21:29Z

reactions/platform/debug-reaction/Services/QueryDebugService.cs

+
+		private readonly ILogger<QueryDebugService> _logger;
+
+		private readonly ConcurrentDictionary<string, Task<QueryResult>> _activeQueries = new();


If we are moving the query result state to the client, why do we need it keep it here too?

I implemented this dictionary for storing the task of initilializing the query results. This dictionary is only used in the InitResult method, which uses the ResultViewClient to get the initial result for a query.

Let us say that we have a query called query1 that is trying to initialize. In the first invocation, the method will check the dictionary. query1 is not present in the dict, it will execute lambda function and store the Task in the dictionary. If there is a second invocation (before the first one completes), it will wait and return the same Task that is in progress.

The downside is that data might have changed in between the two invocations, but the second invocation will not reflect the new changes. Thoughts?

What if we did not store the result set on this tier at all, the front end could invoke a route that proxies a request for the initial query result through to the view service.

Good point. Let me update this

danielgerlag · 2025-03-10T17:21:56Z

reactions/platform/debug-reaction/Services/QueryDebugService.cs

+
+		private readonly ConcurrentDictionary<string, Task<QueryResult>> _activeQueries = new();
+
+		private readonly LinkedList<JsonElement> _rawEvents = new();


Do we need to keep this data here?

ruokun-niu added 11 commits February 7, 2025 14:42

wip

eeaa85a

nit

cb98f9f

react

2de8ba0

wip

b497020

basic working

d0fb6fd

event stream adn query status

c76b4ce

event stream done and added basic css

f7efcf1

updated file structure

5f3d793

Revert "updated file structure"

e72af17

This reverts commit 5f3d793.

updated file structure

4394cc9

sdk working

c46e8d1

github-advanced-security bot found potential problems Feb 19, 2025

View reviewed changes

ruokun-niu added 3 commits February 18, 2025 19:08

final

2fec744

updated reaction provider

5e613cc

nit and license

ec2d2de

ruokun-niu marked this pull request as ready for review February 19, 2025 03:21

ruokun-niu requested a review from a team as a code owner February 19, 2025 03:21

Merge branch 'main' into review-debug-reaction

ce914b2

github-advanced-security bot found potential problems Feb 19, 2025

View reviewed changes

ruokun-niu added 2 commits February 19, 2025 09:34

updated makefile and github workflows

fd70a9c

rebase

f7ae01b

ruokun-niu marked this pull request as draft February 19, 2025 18:38

danielgerlag requested changes Feb 19, 2025

View reviewed changes

ruokun-niu added 2 commits February 19, 2025 13:16

css for sidebar and eventstream

a458f80

updated webapplication in debug reaction and SDK

b8c7eb1

github-advanced-security bot found potential problems Feb 20, 2025

View reviewed changes

reactions/platform/debug-reaction/debug-reaction.Server/Program.cs Fixed Show fixed Hide fixed

ruokun-niu added 2 commits February 20, 2025 13:17

logging update

7399050

updated GET for the queries

c0705db

github-advanced-security bot found potential problems Feb 20, 2025

View reviewed changes

use resutlviewclient from the sdk

b06abaa

ruokun-niu added 3 commits February 20, 2025 16:55

query container id update

6b4f8db

updated to use strongly typed models

7936591

interface for broadcaster

31e3559

ruokun-niu marked this pull request as ready for review February 21, 2025 04:07

ruokun-niu requested review from danielgerlag, amansinghoriginal and agentofreality February 21, 2025 04:07

ruokun-niu added 3 commits February 20, 2025 20:09

nit

22198b1

remove comments

da796c3

updated source rust dockerfiles

5e59489

danielgerlag reviewed Feb 28, 2025

View reviewed changes

ruokun-niu added 14 commits February 28, 2025 13:09

dotnet format

380ba98

moved classes to separate files

0f70a2b

removed unused ExecuteAsync

86dcb38

moved the stream state to the frontend

ae5842c

moved queryreslt state to frontend;

8bf1c6a

updated the mechanism for clearing the query reselt

5ffb8f4

created websocketcontroller

917a363

combined frontend and backend into one container

499521b

updated the logic for update and delete in the frontend

007ccea

updated serialization

d29a7e3

updated image ame

84d6ecc

nit

2330f48

Merge branch 'main' into review-debug-reaction

984c2ff

updated build-test

14f8ab8

github-advanced-security bot found potential problems Mar 4, 2025

View reviewed changes

ruokun-niu requested a review from danielgerlag March 4, 2025 23:00

danielgerlag reviewed Mar 10, 2025

View reviewed changes

removed the unused list for raw events in the backend

38f53db

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Review debug reaction #155

Review debug reaction #155

ruokun-niu commented Feb 19, 2025 •

edited

Loading

danielgerlag Feb 19, 2025

ruokun-niu Feb 20, 2025 •

edited

Loading

danielgerlag Feb 28, 2025

ruokun-niu Mar 4, 2025

danielgerlag Feb 28, 2025

ruokun-niu Mar 3, 2025

danielgerlag Mar 4, 2025

ruokun-niu Mar 4, 2025

ruokun-niu Mar 4, 2025

danielgerlag Mar 10, 2025

ruokun-niu Mar 10, 2025

danielgerlag Mar 11, 2025

ruokun-niu Mar 11, 2025

danielgerlag Mar 10, 2025

ruokun-niu Mar 10, 2025



		var port = Environment.GetEnvironmentVariable("PORT") ?? "8080";
		server.Urls.Add($"http://0.0.0.0:{port}");


		private readonly ILogger<QueryDebugService> _logger;

		private readonly ConcurrentDictionary<string, Task<QueryResult>> _activeQueries = new();


		private readonly ConcurrentDictionary<string, Task<QueryResult>> _activeQueries = new();

		private readonly LinkedList<JsonElement> _rawEvents = new();

Review debug reaction #155

Are you sure you want to change the base?

Review debug reaction #155

Conversation

ruokun-niu commented Feb 19, 2025 • edited Loading

Description

Type of change

Choose a reason for hiding this comment

ruokun-niu Feb 20, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ruokun-niu commented Feb 19, 2025 •

edited

Loading

ruokun-niu Feb 20, 2025 •

edited

Loading