SEZ9 commented on PR #10808: URL: https://github.com/apache/seatunnel/pull/10808#issuecomment-5707598363
@davidzollo thanks for bringing the branch up to date with `dev` at `cd8c818017a` and for the clear description of the conflict resolution and the bootstrap failure-path cleanup in the IT. Looking at what actually changed in `cd8c818017` relative to the previous tip `ea6aa5d15b`, the merge itself did not touch the files behind the earlier findings — `ServerExecuteCommand.java` in particular came through as a trivial merge from the PR side. So the open items from the previous round still stand as they were; nothing there was closed or invalidated by the rebase. To get this over the line, could you address (or explicitly reply to) the following: 1. **Fallback coordinator selection in `ServerExecuteCommand`** (F1/F2): the client-side "first non-lite member" guess can disagree with the server's actual active coordinator. Either resolve the active coordinator from the server-side source of truth, or tell me why the guess is acceptable for the CLI use case and document that limitation. 2. **Unknown-coordinator output** (F6): when no active coordinator can be resolved, the member list currently shows every non-lite member as `MASTER`. Please mark the coordinator as unknown/unresolved instead of silently labelling all of them. 3. **`incompatible-changes.md`** (F3/F7): please add the CLI member-list "ACTIVE MASTER" semantic change for separated clusters, and state the standby/failover window in which no node reports as active coordinator for `isMaster`/`cluster_info`. 4. **`telemetry.md`** (F4): the "bounded timeout" for the final cancel-time metrics flush should state the actual value and whether it is configurable. 5. **`rest-api-v1.md`** (F5): a short note that the new `nodeRole`/`coordinator`/`worker` fields expose cluster topology on an endpoint that ships without authentication by default would be good. 6. **`ServerExecuteCommandTest.java`** (F8): replace the inline `java.util.Collections` fully-qualified reference with an import. Since you mentioned no local compile, unit test, or container E2E was run for this head, I'll hold off on a final pass until the queued checks on `cd8c818017a` have finished. If any of the above were already handled somewhere I missed, just point me at the spot and I'll re-check. <!-- streview-comment:1109 --> -- 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]
