SiriusTL666 commented on PR #12974:
URL: https://github.com/apache/seatunnel/pull/12974#issuecomment-6399225991
Thanks for the careful re-read at the exact head, and for correcting your
own Issue 3 wording — that is more useful than leaving it as optional.
**Issue 3 (Medium, blocking) — done.**
Added an entry under `## dev` in both
`docs/en/introduction/concepts/incompatible-changes.md` and the ZH equivalent,
at `fcac22db`. It states:
- old vs new default (no application-level timeout → 10000 ms),
- that this affects existing configs which do not set the option,
- source scope and sink scope,
- that the source read default is unchanged (`read_timeout_ms = 0`, idle
streams are not failures),
- `connect_timeout_ms = 0` as the explicit opt-out.
You are right that the connector table alone is not an upgrade warning — an
operator upgrading without re-reading the table would get a new failure and no
compatibility guidance.
**Optional coverage — also added, since it was cheap.**
- `testSinkClientPassesConnectTimeoutToSocket`: asserts `SocketClient`
passes the configured value to `Socket.connect` (via `mockConstruction`), so
the sink path is pinned the same way as the source.
- `testSinkClientClosesSocketWhenConnectFails`: asserts the socket is closed
when connect throws.
`mvn -pl seatunnel-connectors-v2/connector-socket test` — **16/16 pass**.
`spotless:check` clean.
**On the failed Build 40195143538 — I agree with your diagnosis, and here is
the evidence.**
Paging the job confirms it:
```
Run / unit-test (11, windows-latest)
4. run all modules unit test completed skipped
5. run all modules unit test (Windows) in_progress None <-- no
conclusion
6. Upload test reports pending None
```
The Windows test step never completed and uploaded no report, which matches
a runner losing communication rather than a failing assertion. I did **not**
patch anything for it, per your advice — a rebase or unrelated source change
would not be an evidenced cure.
I have re-triggered Build on the new head `fcac22db` after rebasing onto
current `dev` (the branch is now up to date, so the earlier 4-behind drift is
gone). I will post the result. If the Windows runner drops again, I will rerun
that job rather than touching code.
**On approval:** understood — your COMMENT is a technical assessment, not a
write-level approval. I will wait for a committer with write access.
--
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]
