lizhimins commented on PR #4688: URL: https://github.com/apache/rocketmq-dashboard/pull/4688#issuecomment-5845586440
Thanks, but we cannot take this one: the branch you are fixing is not reachable in production. `AdminClient.getTopic(String)` is declared at `provider/apache/AdminClient.java:29` and implemented at `RocketMQAdminClientImpl.java:107`, but nothing calls it. Only two classes hold an `AdminClient` — `MetadataService.java:83` and `ApacheInstanceProvider.java:56` — and neither invokes `getTopic`: `MetadataService` calls `getConsumerGroup` (`:418`), `getConsumerGroupSettings` (`:520`) and `updateConsumerGroupSettings` (`:527`); `ApacheInstanceProvider` calls the topic/group CRUD plus `previewResetOffset`, `resetOffset` and `sendMessage` (`:110`-`:226`). The only callers of `getTopic(String)` anywhere in the tree are the two cases in `RocketMQAdminClientImplTest`. The user-visible behaviour you want already exists: topic detail goes through `MetadataService.getTopic(instanceId, clusterId, name)` (`:130`), which throws `BusinessException(404, "Topic not found: " + topicName)` at `:133`. Your new test passes only because it drives the implementation class directly. The grading itself is right in principle — `MetadataService.java:239` already maps `TOPIC_NOT_EXIST` for `examineTopicStats` — so if you would like to contribute here, the useful follow-up is different: either wire `AdminClient.getTopic` to a real caller, or delete the dead method. Please open an issue first so we can agree which we prefer before you write code. Two mechanical notes if you resubmit: the branch is stale (`RocketMQAdminClientImpl.java` has moved +268/−176 since your base), and the new test has no blank line before `@Test` while leaving two blank lines after it. -- 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]
