Feature/2634/simple html node monitor #3000
Conversation
|
|
|
Looks great! Could we change the algorithm for coloring for certain fields?
Let me know if you can do that in this PR or prefer a new one. |
|
I'll take care of it in this PR; it shouldn't be a big deal. I just don't know if I'll get around to it in the next few days. |
* Rebase/Merge completed due to REST API reorg * ReportDiffs: Compares reports and highlights deviations based on thresholds. * NodeType/UserName: Displays node type and username alongside the address. * Unified BE/FE naming conventions and API endpoints for consistency. --- ReportDiffs enables comparison across all reports and provides average values. It supports three configurable thresholds, which can be adjusted in the settings. Threshold violations are visually highlighted: for subtables, the background of the button that toggles the detail view is marked based on the highest exceeded threshold.
bsc7
force-pushed
the
feature/2634/Simple_HTML_NodeMonitor
branch
2 times, most recently
from
a26d4e5 to
e5d4099
Compare
|
Branch rebased onto new main. Conflicts have been resolved.
|
apps/rest-api-app/build.gradle.kts
Outdated
| @@ -1,5 +1,6 @@ | |||
| plugins { | |||
| id("bisq.java-library") | |||
| id("bisq.protobuf") | |||
There was a problem hiding this comment.
Why is that needed? We don't have protobuf files here.
There was a problem hiding this comment.
Ah saw now you added a protobuf file for AddressDetails.
What is the intention? We use protobuf only or p2p network communication. Here it seems it is just a dto for the response. If so, I would suggest to remove the protobuf part and move the file into report package.
There was a problem hiding this comment.
Ah, okay. I wasn't aware that Protobufs are primarily for P2P communication. I added them because you mentioned we should avoid using DTOs as much as possible. Since I needed one, and Protobufs seemed like the "state of the art" solution to me as a beginner, I implemented it that way.
But regarding the solution:
I would suggest to remove the protobuf part ...
You mean I should remove the line id("bisq.protobuf") from build.gradle.kts, which means Protobuf won't be generated, and as a result, AddressDetails would need to be converted to AddressDetailsDto.
Or should Protobuf still be used? If so, I'm not very familiar with the build process: where exactly should I then define the usage of Protobuf(id("bisq.protobuf"))?
I would suggest to ... move the file into report package.
You mean placing the transport class (whether Protobuf or DTO) alongside ReportRestApi, or using a separate package for transport classes alongside ReportRestApi? I find the latter option more elegant.
There was a problem hiding this comment.
Yes the id("bisq.protobuf") from gradle and rest_api.proto file can be removed as well as the implements NetworkProto and the 3 PB related methods in the AddressDetails class. The void verify() can be kept if you like, but its also mainly for network data to protect against ddos.
And move that class to bisq.rest_api.report.
| } | ||
|
|
||
| @Override | ||
| public bisq.rest_api.protobuf.AddressDetails toProto(boolean serializeForHash) { |
There was a problem hiding this comment.
Best to use import to avoid the long name space.
There was a problem hiding this comment.
It would result in a name collision since the classes have the same name, which makes the import impossible. Unless, of course, I misunderstood what you meant. :)
There was a problem hiding this comment.
I just tried on your branch with import bisq.rest_api.proto.AddressDetails; and it worked.
There was a problem hiding this comment.
I guess the truth lies somewhere in the middle :))
NodeMonitor: FE: +ReportDiffs, +NodeType/UserName
ReportDiffs enables comparison across all reports and provides average values. It supports three configurable thresholds, which can be adjusted in the settings. Threshold violations are visually highlighted, hover shows the specific average value and the amount by which it was exceeded. For subtables, the background of the button that toggles the detail view is marked based on the highest exceeded threshold.
Origin: #2634, Previous PR: #2978