SEZ9 commented on PR #12298: URL: https://github.com/apache/seatunnel/pull/12298#issuecomment-5852160593
Checked the actual current diff at `6f00c681b385` (rebased onto `dev@025d08f9bcb0`), rather than the older head cited in the question: - **F1/F2/F3:** [JettyService](https://github.com/apache/seatunnel/blob/6f00c681b3856ec3c93b040385cfeb0dde0bd354/seatunnel-engine/seatunnel-engine-server/src/main/java/org/apache/seatunnel/engine/server/JettyService.java#L110) keeps the member's `ServerConnector` and reads its bound `getLocalPort()`; it does not write the selected port into `HttpConfig`. `HttpConfig.getPort()` still means configured port. The port field has no bound-port Javadoc, and the [English](https://github.com/apache/seatunnel/blob/6f00c681b3856ec3c93b040385cfeb0dde0bd354/docs/en/engines/zeta/rest-api-v2.md#L81) / [Chinese](https://github.com/apache/seatunnel/blob/6f00c681b3856ec3c93b040385cfeb0dde0bd354/docs/zh/engines/zeta/rest-api-v2.md#L76) pages explicitly say the configuration remains unchanged. - **F4, precise scope:** HTTP-disabled construction does not probe or create an HTTP connector, and the HTTP-started log is emitted only after `server.start()` with an HTTP connector present. The integer port getter **does retain the existing configured-port fallback** before startup/when HTTP is disabled; it is not a new “no HTTP port” sentinel. The HTTPS-only fixture explicitly covers that fallback. This PR does not implement HTTPS peer fan-out. I clarified this in the description so it does not imply otherwise. - **F5/F7:** The description now explicitly identifies the Jetty startup message; it does not cover `ClientExecuteCommand`'s local-mode message. The constructor comment names `LogService`/`LoggerLevelService` and explains why shared `HttpConfig` cannot hold each member's runtime port. - **F6/F8:** [testDynamicHttpPortIsResolvableByPeers](https://github.com/apache/seatunnel/blob/6f00c681b3856ec3c93b040385cfeb0dde0bd354/seatunnel-e2e/seatunnel-engine-e2e/connector-seatunnel-e2e-base/src/test/java/org/apache/seatunnel/engine/e2e/RestApiIT.java#L254) asserts the shared config object, configured port `8080`, distinct bound ports, the shared canonical log directory and an actual job log before checking both node IDs from `/logs`. Cluster logger assertions also check both runtime node IDs. Current-head local Spotless/full-reactor verify and the 3 operation tests plus the real `testLoggers` integration test passed, as recorded in the description. CI is not green: attempt 5 retains 78 successful jobs and these two failures: [OceanBase/Java 8](https://github.com/SEZ9/seatunnel/actions/runs/36242438663/job/108524509181), [Java 11](https://github.com/SEZ9/seatunnel/actions/runs/36242438663/job/108524509197). Both report `NotSerializableException: io.debezium.relational.TableId`. Independent #12489 fixes the duplicate Debezium packaging; its OceanBase compatibility tests passed 5/5 on each JVM, and both containing shards are now successful. That dependency is still unmerged and has not been copied into this feature PR. -- 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]
