SEZ9 commented on PR #12011:
URL: https://github.com/apache/seatunnel/pull/12011#issuecomment-5564708022
Thanks @DanielLeens for the status summary, and @goutamadwant for the ping.
Good to see the `Build` run (`33579103208`) is green on `cc7658b6`, and that
the `community` credential masking, the unwrapped `SnmpTargetFactory`
construction exception, and the e2e coverage gap have been verified against the
current head.
Before the final merge call, a few points from my earlier review haven't
been explicitly confirmed in this thread. @goutamadwant, could you confirm (or
point me at the change) for each:
1. **Defaults parity in `SnmpOptions`** — the port `161`, timeout `5000` ms
and `retries = 1` defaults in the shared `SnmpOptions` match the pre-existing
`SnmpSourceOptions` values exactly, so existing SNMP source jobs behave
identically after upgrade.
2. **Blocking retries in `write()`** — with synchronous per-row SET and
SNMP4J retries, an unreachable agent blocks each write for the full timeout
across all retry attempts. Is there anything bounding this, or at least a note
in the sink docs about tuning `timeout`/`retries` on streaming engines to avoid
checkpoint timeouts?
3. **Duplicate delivery via retransmission** — with `retries = 1` by
default, SNMP4J may retransmit a SET PDU independently of checkpoint replay. A
short sentence in the sink docs calling this out would be enough.
4. **Sink docs option table** — please add the standard common-options row,
since the example itself relies on `plugin_input`.
5. **`community = ${SNMP_COMMUNITY}` in the docs example** — either use a
literal placeholder value or add a line explaining how the variable is
supplied, so the example doesn't fail HOCON resolution out of the box.
If 1 is already handled and the rest are just doc touch-ups, I'm happy to
proceed once those land — no need for another full review round on my side.
<!-- streview-comment:869 -->
--
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]
