DanielLeens commented on PR #10808:
URL: https://github.com/apache/seatunnel/pull/10808#issuecomment-5775056446

   Thanks for following up, @SEZ9. Since I already pulled the full `0d4d8624c` 
diff for my last review rather than going off the commit message, here are 
direct pointers to the four hunks you asked about, so you don't have to wait on 
a summary:
   
   1. `docs/en/introduction/concepts/incompatible-changes.md`, in the 
master/worker separation entry: it now has a dedicated `CLI member list` bullet 
explaining that `ACTIVE MASTER` marks the active SeaTunnel coordinator (not the 
Hazelcast master), plus the exact `Active master: <address> (best effort)` and 
`Active master: UNKNOWN` wording, and a separate `Failover window` bullet 
stating that while a coordinator is being elected (or none is visible), no node 
reports `isMaster=true` and `cluster_info` isn't exported, with a note that 
alert rules must tolerate that window. Both (a) and (b) are covered, in English 
and Chinese.
   
   2. `docs/en/engines/zeta/telemetry.md`: the final cancel-time metrics flush 
section now states "a bounded timeout of 1 second. This value is fixed and not 
configurable." — an explicit hard-coded number, not a vague "bounded timeout."
   
   3. `docs/en/engines/zeta/rest-api-v1.md`: right after the 
`nodeRole`/`coordinator`/`worker` field descriptions there's now a one-line 
note that these fields disclose cluster topology and that "The REST API V1 has 
no authentication, so restrict network access to it when this information is 
sensitive."
   
   4. `ServerExecuteCommandTest.java`: `java.util.Collections` is a proper 
top-of-file import now, and both call sites use the short 
`Collections.singletonList(...)` form — no inline fully-qualified references 
remain.
   
   I independently verified all four directly against the diff, so you should 
be good to re-check and move to approve on the doc/CLI side.
   
   One live-status update since my last review: the fork's `Build` run for this 
exact head (`0d4d8624c`) has now finished (it was stuck queued when I last 
looked) and finished red. The one real failure is 
`TaskDeploymentPrePublicationClassLoaderLeakIT.testFailureBeforeContextPublicationReleasesClassLoadersAndReportsFailure`,
 which is a pre-existing `dev` test added by an unrelated PR (#12143) — this 
PR's diff doesn't touch that file at all, so this looks like an 
upstream/environmental failure rather than something introduced here. 
Separately, the branch is now `diverged` from `dev` by a much wider margin than 
last round (`ahead_by=36`, `behind_by=35`, `mergeable=false`), so a real merge 
conflict has opened up since my last pass. I'd recommend syncing with the 
latest `dev` next, both to pick up whatever fixed/changed that test upstream 
and to clear the conflict, then re-running CI on the synced head.
   


-- 
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]

Reply via email to