lizhimins commented on PR #6639:
URL:
https://github.com/apache/rocketmq-dashboard/pull/6639#issuecomment-6190310087
Closing this one: The `dlq_check.sh` finding is real and the fix is
well-scoped: `RocketMQDLQProvider.classifyOutcome` can return a non-`SUCCESS`
outcome (and `scanIncomplete=true` from the `!foundAll` path) while the
endpoint still returns code 200, so the old `assert body.get("code") == 200`
gate really could print `PASS 死信链路` after resending only 2 of 3 messages.
Switching from `assert` to an explicit `RuntimeError` is also correct, since
`python -O` strips asserts. The seven unittest cases map one-to-one onto the
new conditions and run the committed script for real, which is the right shape.
Two blocking problems, and one question for us to settle: 1. Please drop
`3e310352b` ("fix(server): serialize the ownership migration within one
process") and the `3569e605a` `ci:` commit. `3e310352b` is the head commit of
#6175 by a different author and lands here as +17 lines in
`server/src/main/java/org/apache/rocketmq/studio/instance/ResourceOwnershipSchemaMigration.java`,
whic
h has nothing to do with the DLQ regression script. We cannot merge another
contributor's open PR through this diff, and #6175 is not going to land as a
prerequisite, so this has to be dropped permanently. The `ci:` prefix should
also be reserved for real CI changes. 2. `.claude/skills/regression-rocketmq/`
is our own internal acceptance harness (added in #6633). It can only be
validated against our private ECS + broker fixture, and CI never runs it — your
own note says the Python tests are not auto-discovered. We are still deciding
whether community changes to that directory are in scope at all, so please hold
this one until we answer; if we do take it, we would want either just the
11-line `dlq_check.sh` change, or the tests plus a CI hook that actually runs
them. 3. If it does go ahead: please also update `SKILL.md` (the resend verdict
is now stricter, and `scripts/tests/` is new), and note the minimum Python
version — `tests/` has no `__init__.py`, so `unittest discover` the
re relies on namespace-package support.
--
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]
