SiriusTL666 commented on PR #13009: URL: https://github.com/apache/seatunnel/pull/13009#issuecomment-6623485114
Thanks for the approval and for catching Issue 1 — you were right, and the PR description has been corrected. I verified the default directly rather than relying on my own commit message: `javap -c` on `SocketClient.class` from commons-net 3.6 shows the no-arg constructor doing `iconst_0; putfield connectTimeout`, so `connectTimeout` is indeed `0` (unbounded), not 60000. Two passages were wrong and both are now fixed: - The "connect step is not the problem" paragraph now states that the connect is unbounded too, and that this PR deliberately scopes itself to the control-channel read. - The testing section no longer claims `connectTimeout` already defaulted to 60000. While updating it I also refreshed the numbers, which had drifted: - 26/26 → **31/31** locally (`SeaTunnelFTPFileSystemTimeoutTest` is now 14, not 9), matching the green `unit-test` lane on this head. - Recorded the new `FtpFileIT.testFtpControlTimeoutIsHonouredThroughTheJobConfig` case and what it asserts. - Corrected the reverse-verification line: reverting `setDefaultTimeout` turns **5** tests red, one of them being the real-socket case reporting that the call was still blocked past the 30 s independent deadline rather than hanging the build. One note on your caveat about `assertOnlyControlTimeoutIsRejected` coupling to the validator's message text: agreed, and that is the intent — it asserts the *whole* failure is exactly `(1 error)` naming `control_timeout` with `type: value`. Asserting only the exception type was the flaw last round, since a missing required option throws the same type. If that message format ever changes, the test should fail loudly rather than silently stop covering the constraint. No code changed in this update, only the description, so the reviewed head `5c268fe1` still stands. -- 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]
