DanielLeens commented on PR #11613: URL: https://github.com/apache/seatunnel/pull/11613#issuecomment-6359796934
Thanks @SEZ9 for the detailed pass on `4aba1e6892`, and @danielnadean and @davidzollo for the quick turnaround. I re-checked each open item against the current head (`4aba1e68924e`, unchanged since my last review) so the list can be narrowed down; this is a source read only, nothing was executed locally. Already present on this head: - **F1**: the `/update-tags` example in `docs/en/engines/zeta/rest-api-v2.md` uses flat `tag1`/`tag2` keys, and the text states values must be strings, nested objects are not supported and no key (including `uuid`/`tags`) is reserved. - **F2**: E2E coverage exists in `RestApiIT` (`testHttpServiceStatusReportsEffectivePortsWithoutSecrets`, `testUpdateLocalMemberTagsOnlyMutatesTheServingMember` which also posts a mismatching UUID, plus legacy `REST_URL_UPDATE_TAGS` calls) and in `BasicAuthenticationIT` for both new endpoints. - **F4**: the `Limitation` subsection of `/update-local-member-tags` documents that the UUID check is local to the serving node and fails behind a load balancer. - **F5**: `/update-local-member-tags` has its own section with auth note, body table, success example and an error table covering the UUID mismatch and non-object `tags` cases. - **F6**: `web-ui.md` (Checkpoints bullet) describes the disabled-until-end-state behavior and the `No checkpoint found` result after cleanup. - **F7**: `/http-service/status` has a per-field table and the example uses 8080. Small genuine leftovers, all documentation and non-blocking: (a) the error table for `/update-local-member-tags` does not list a non-string tag value case, so please add it if the server rejects it (or state how it is handled); (b) F8 screenshots for the new UI surfaces are still outstanding, which I treat as Low severity, as in my last review. The real gate is unchanged: the PR is still a Draft, and the `Build` check is `ACTION_REQUIRED` (https://github.com/apache/seatunnel/runs/119921964326) because no workflow run exists on the fork, so none of the new server, E2E and UI tests have run in CI. To pass the automated CI checks for this PR, could you please enable CI workflows in your forked repository first? You can refer to this check run for the required configuration: https://github.com/apache/seatunnel/runs/119921964326. If you run into any trouble enabling CI, feel free to leave a comment here. Once a full `Build` result exists I will compare any failure against the diff. -- 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]
