SEPURI-SAI-KRISHNA commented on PR #12438:
URL: https://github.com/apache/seatunnel/pull/12438#issuecomment-5882867817
Thanks @SEZ9, and agreed on keeping the `continue` change out. I will file
the issue and the PR for it once this lands, with the two-topic
`RocketMqAdminUtilTest` case where the first topic returns stats and the second
raises `TOPIC_NOT_EXIST` with the route available, so the earlier topic's
offsets have to survive.
On the truncation: the comment posted in full on my side, 500 words, and the
sentence you saw cut is complete in it:
> A comment anchored on a PR number and on what the code used to be is the
same pattern that went stale and created this PR, so I dropped the history
rather than updating it.
So nothing is missing, and there was no further content beyond what you
already read. Something in between clipped it.
One thing worth flagging before you check the diff: **the head has moved
from `533cfde8` to `5cc724b9342`**. That is only a merge of current `dev`,
taken to pick up #12444, which fixes the `PayPalClientTest` Windows flake that
was reddening this PR. The two files you are reviewing are byte-identical
across the two heads:
| file | blob at `533cfde8` | blob at `5cc724b9342` |
| --- | --- | --- |
| `RocketMqAdminUtil.java` | `03ecf079` | `03ecf079` |
| `RocketMqIT.java` | `7fc989a0` | `7fc989a0` |
So reviewing at either head gives the same text, and the diff is still
comment-only: stripping comments leaves both files identical to `dev`.
On the two points you said you would verify, both hold at the current head:
- `RocketMqIT.java:535-540` no longer references a PR number and no longer
narrates what the code used to be. It reads: "currentOffsets raises
RocketMqConnectorException when the lookup itself fails, and returns an empty
map only when the group has legitimately committed nothing, so matching that
exception here retries transient metadata failures without masking a genuine
cold start. The empty-map case is what the getMinOffset() fallback below
covers."
- `RocketMqAdminUtil.java:349-359` names the consequence as a full rewind:
"The caller reads an empty map as a cold start and rewinds every topic to its
first offset, so supporting that layout means turning this into a continue that
keeps the earlier offsets." That matches the `if (groupOffsets.isEmpty()) {
listOffsets(queues, CONSUME_FROM_FIRST_OFFSET) }` at
`RocketMqSourceSplitEnumerator.java:380-383`.
CI note so the red badge does not hold this up: the run on the current head
is void rather than failing. `Code style` died at 4m58s before Spotless ever
ran, because the Maven wrapper jar download from `repo.maven.apache.org`
returned HTTP 429 on five consecutive retries and the job ended in `Could not
find or load main class org.apache.maven.wrapper.MavenWrapperMain`. That gated
52 jobs. I am rerunning it; `spotless:check` is clean locally on both modules.
--
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]
