unbridled-41 commented on PR #5090:
URL:
https://github.com/apache/rocketmq-dashboard/pull/5090#issuecomment-5827779243
### Issue linkage — none filed for this change
Unlike the other three PRs from this batch, I did not open an issue for this
one, because the change
contradicts an expectation the repository has already recorded for this
exact method and I do not
want to file a report asserting a defect the project may have specified on
purpose.
- #1583 ("Paginate Tencent Topic subscriptions") is about
`TencentInstanceProvider#getTopicConsumers`
— the method this PR changes — and lists, as expected behaviour, fetching
pages "until one of these
conditions is met: the response page is empty or shorter than the
requested limit; the collected
count reaches `SubscriptionCount`; …". The current code implements exactly
that: a short page ends
the scan before the count is consulted. The merged fix for #1583 is where
this loop came from.
- #4530 / #4531 ("reject incomplete resource pages") then applied the
fail-closed total check to
three *other* inventories — `listTopics`, `listConsumerGroups` and the
group subscription read —
and left this one out.
So this PR removes a stop condition that #1583 recorded for the same method,
in favour of the
stricter contract the sibling scans now follow. Both readings are
defensible; I cannot tell which one
is wanted, so I am leaving the decision here instead of asserting a defect.
If the short-page stop is
intended for this path, this PR should be closed rather than reviewed.
The behaviour itself is real, not speculative — the description has the
measurement (a page of 40
rows against `SubscriptionCount = 250` returns 40 consumers with no error).
The open question is only
whether it should fail closed or stay as specified in #1583.
--
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]
