SEZ9 commented on PR #10900: URL: https://github.com/apache/seatunnel/pull/10900#issuecomment-5476203023
Thanks @DanielLeens for the careful trace-through on head `7d2cd7b3ff32` and @goutamadwant for the status ping — the thread has been quiet since the last review, so a status update from the author would be very helpful. To keep this actionable, here is where the review stands and what remains before merge: **Issue 1 (unauthenticated `0.0.0.0` listener) — blocking.** Since `connector-edge-socket` already establishes the pattern of requiring a `token` option with an `__AUTH__:<token>` handshake despite binding `0.0.0.0` by default, this connector should follow suit. Concretely, please either: - add an optional shared-secret/allowlist mechanism mirroring the EdgeSocket pattern, **or** - change the default bind to loopback with an explicit opt-in for wider exposure, plus a prominent security warning in the docs. **Issue 2 (checkpoint coupling / delivery semantics) — documentation ask.** As @DanielLeens confirmed, `SyslogSourceReader` inherits the no-op `snapshotState`/`restoreState` defaults, so rows still in the OS socket buffer at failure time are lost — a real at-most-once gap. The parallelism sub-claim is already covered upstream by the `checkArgument` in `AbstractSingleSplitSource.createReader`, so no code change is needed there. The remaining ask is to document the at-most-once delivery semantics explicitly in the connector docs so users aren't surprised by data loss on task failure. If the original author is unable to continue, please let us know so the community can either pick this up or update the tracker accordingly. Happy to help review a follow-up commit addressing the two items above. <!-- streview-comment:691 --> -- 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]
