DanielLeens commented on PR #12298: URL: https://github.com/apache/seatunnel/pull/12298#issuecomment-5812776938
Thanks for pulling this together, @SEZ9 — this matches my own review on this head (`d1cf006305aa`) exactly: Issue 1 (bound port written into the shared `HttpConfig` bean, confirmed against `ServerExecuteCommandTest.testMemberList`) is the one blocking item, and the rest — the HTTPS-only guard, the two doc updates (including the now-false `/loggers?scope=cluster` limitation sentence), and the `RestApiIT` assumption/naming cleanups — are Low and can land in the same push. Nothing to add on my side. One thing worth flagging since my last review: the apache-side `Build` check on this head now shows `FAILURE` (it was still `in_progress`/`queued` when I reviewed a few hours earlier). That check has been a pass-through pointer to the fork's own run in every previous round rather than a real signal on its own, so before anyone reads this as a new blocker — could you confirm whether the fork's actual run for `d1cf006305aa` is green, or whether something changed there? If it's the same pre-existing `all-connectors-it-2` (#12344) situation as before, that doesn't change anything about this PR's merge readiness; if it's something new, I'd want to know before the next re-review. I'll take a fresh full pass as soon as Issue 1 is addressed — happy to review either the per-node-state approach or the document-and-test-the-shared-config-assumption alternative, whichever direction you go. -- 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]
