SEZ9 commented on issue #11007: URL: https://github.com/apache/seatunnel/issues/11007#issuecomment-5923555691
@Nikk8091 thanks for the update on the Socket Source slice — that matches the scope that was confirmed above: declarative nonblank `host` and positive `port` validation only, with address parsing, defaults, framing, and the Sink path left untouched. The test matrix you describe (valid and padded host, each option missing, blank/whitespace host, zero/negative port) covers what was asked for. A few things to make the review on the PR quick: 1. Please add the "rule removed → regression test fails" evidence to the PR description itself (e.g. the failing assertion output for the blank-host and non-positive-port cases with the condition temporarily removed), so a reviewer doesn't have to reproduce it locally. 2. Confirm in the PR that the diff is limited to the Socket Source factory `optionRule()`, its factory tests, and the EN/ZH source docs — no changes to the Sink factory, the TCP reader, or connection handling, as you state here. 3. For the padded host case, please state explicitly in the test what the expected behavior is (accepted as-is, no trimming), since the agreed scope is validation only and not normalization. Once those are in the PR I'll take a look there. Please keep any further Socket discussion on the PR rather than in this tracker. <!-- streview-comment:1437 --> -- 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]
