lizhimins commented on PR #4975:
URL:
https://github.com/apache/rocketmq-dashboard/pull/4975#issuecomment-5806622000
Closing as a duplicate of what the branch already ships, plus it does not
compile.
1. `BrokerConfigDiffService` already does this. Its `COMPARED_FIELDS`
(`BrokerConfigDiffService.java:37-46`) covers every property this PR audits —
`flushDiskType`, `autoCreateTopicEnable`, `autoCreateSubscriptionGroup`,
`maxMessageSize`, `brokerPermission`, `fileReservedTime` — and more
(`msgTraceTopicName`, `deleteWhen`, `writeQueueNums`, `readQueueNums`).
`compare(clusterId, instanceId)` already pulls live per-broker configs, reports
`driftDetected`, `complete`, `brokerCount` vs `reachableBrokerCount`,
per-broker reachability with the failure message, and per-field per-broker
values (`BrokerConfigDiffVO.java:32-70`), and `ClusterController.java:85`
already exposes it. The only genuinely new content here is a consistency score
and severity labels; those belong on the existing VO, not on a second service
method, second VO family and second endpoint.
2. The new path is also strictly less informative: `catch (Exception
ignored) {}` swallows broker-config failures and `totalBrokers`/`onlineBrokers`
are both set to the number of brokers whose config was successfully read, so an
unreachable broker disappears from the report instead of being reported
unreachable.
3. Compile errors: `ClusterConfigVO` has no `getBrokerName()`
(`ClusterConfigVO.java:29-39`); `maxMessageSize`, `fileReservedTime` and
`brokerPermission` are primitive `int`, so `!= null` is illegal; and
`extractValue`'s switch mixes a `FlushDiskType` enum arm with `String` arms.
4. The score formula `100.0 - driftedCount * 16.6` has no stated basis, and
all four `BrokerConfigDriftEngineTest` methods are named `testXxx` instead of
the `...Test` suffix; they build fixtures on the imagined VO shape, so they
cannot compile either.
5. `GET /api/clusters/{id}/config-drift-audit` is not added to
`docs/api-spec.md`.
If you want severity and a health score for config drift, please propose
them as additional fields on `BrokerConfigDiffVO` produced by the existing
`compare` path, keeping the reachability reporting intact.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
