DanielLeens commented on PR #12011:
URL: https://github.com/apache/seatunnel/pull/12011#issuecomment-5578521507
@SEZ9 thanks for flagging both points.
**1. The "cut off" spot:** confirmed via the raw API response, the review
body is intact (15580 characters, ends cleanly at the merge recommendation) —
this is a rendering glitch, same as the one we ran into on #11932. The sentence
you're seeing truncated is the opening of section 1.1:
"I diffed the last-approved head (`cc7658b6`) against the current head
(`f74a0d1609c`) directly and verified every hunk against source rather than
trusting the PR-comment summaries:"
— followed by a bulleted list of the four changed files (`SNMP.md` EN/ZH,
`SnmpSinkFactory.java`, `SnmpSinkConfigTest.java`, `SnmpSinkFactoryTest.java`),
then the rest of 1.1 through section 5 as normal. A reload should pick up the
rest, same as last time.
**2. Per-item confirmation on `f74a0d1609c`** — all verified as addressed,
none deferred:
- `SnmpTargetFactory` host/UDP address handling (unwrapped
`IllegalArgumentException` + socket leak): fixed and re-verified at this head.
- Synchronous per-row SET blocking on unreachable agents: documented (the
`timeout_millis * (retries + 1)` formula), still present at this head.
- Missing e2e case for the sink via `plugin-mapping.properties`: fixed and
re-verified — the e2e module is registered and exercises the real SPI/engine
path.
- `common-options` table row: now a real table row, not the old footer-link
presentation.
- `${SNMP_COMMUNITY}` example: replaced with a literal placeholder plus
explanatory prose, so the example no longer fails HOCON resolution out of the
box.
- Retries/duplicate-delivery note (SNMP4J retransmission on non-idempotent
OIDs): documented, still present at this head.
Nothing on that list was pushed off as a follow-up — all six are closed on
the current head.
On CI: it's now green — `Build` shows `pass` (11h7m) on `f74a0d1609c`, which
was the one blocker I had left open. From my side there's nothing further
outstanding; ready for a write-access maintainer to make the merge call.
--
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]
