SEZ9 commented on PR #12298: URL: https://github.com/apache/seatunnel/pull/12298#issuecomment-5738758603
Quick status on the open items so we're aligned before the next push: **Blocking** - **Shared mutable `HttpConfig` (JettyService.java)** — this is still the one item gating approval. Please move the bound-port state out of the shared config bean and keep it per node (e.g. on the `JettyService` instance / the member that actually bound the port), so multiple members built from one `SeaTunnelConfig` don't all advertise the last-bound port. Once that refactor lands I'll re-review right away. **Follow-ons (small, can go in the same push)** - **Disabled HTTP connector (JettyService.java)** — guard the dynamic-port selection and write-back so it only runs when the HTTP connector is actually enabled; in HTTPS-only mode we shouldn't advertise a port nothing listens on. - **`HttpConfig.getPort()` semantics (HttpConfig.java + REST docs)** — the "bound port instead of configured port" change is currently only in the field Javadoc and the PR description. Please reflect it in the REST docs as well. - **PR description** — either trim the claim about the fix's reach or also update the local-mode REST log line in `ClientExecuteCommand`, which still prints the configured port. - **RestApiIT** — make the two implicit assumptions explicit: (a) both members write to one shared log directory, and (b) node2 inherits port 8080 from the test `seatunnel.yaml` in `testDynamicHttpPortIsResolvableByPeers`. A short assertion/comment for each is enough so a layout change doesn't silently break the test. - **Inline comment (JettyService.java)** — the hoisted `HttpConfig` local is good; just point the comment at `LogService`/`LoggerLevelService` so the next reader finds the callers without grepping. Nothing else from my side. Ping me when the per-node bound-port refactor is up and I'll take a fresh look. <!-- streview-comment:1161 --> -- 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]
