SEZ9 commented on PR #11613: URL: https://github.com/apache/seatunnel/pull/11613#issuecomment-6407960702
Thanks for bringing the branch up to date. The head moved from `379efa74` to `117cf5289a0bce0767f5a89a68de1594fd731bec` with only a merge of `dev`, so the earlier review points still apply. Here is what I'd still like to see: **PR11613-F1 (Docs)** — Please pick a key name in the legacy `/update-tags` example that doesn't suggest nested object values are accepted, and add a sentence stating that the contract is a flat map of string keys to string values. **PR11613-F2 (Test)** — The unit tests are useful, but please add RestApiIT/E2E coverage that (a) calls `/update-local-member-tags` on the serving member and on a non-matching UUID, (b) hits `/http-service/status`, and (c) asserts the legacy `/update-tags` behaviour is unchanged through the actual HTTP path. **PR11613-F3 (Security)** — Please confirm, with a test or by pointing at the registration, that both new servlets sit behind the same basic-auth/mTLS handler chain as the existing endpoints. A negative test (unauthenticated request rejected) for `/update-local-member-tags` would be ideal since it mutates state. **PR11613-F4 (Functional)** — The local-only UUID check means the UI can't tag a worker when traffic goes through a load balancer/reverse proxy or when only the master exposes the Web UI. Please either document this limitation clearly in the endpoint section, or describe how you intend to handle it (e.g. a follow-up). **PR11613-F5 (Docs)** — `/update-local-member-tags` needs its own section in rest-api-v2.md, with request body, success response, and the 400 cases (non-object `tags`, non-string values, UUID mismatch), rather than being described only in prose inside the `/update-tags` section. **PR11613-F6 (Robustness)** — Please document in the "Restore Latest State" section of web-ui.md how restore behaves when the source job is still RUNNING, and what the user sees when the source job's checkpoint state has already been cleaned up. **PR11613-F7 (Docs)** — Please add per-field descriptions for the `/http-service/status` response and switch the example port to 8080 for consistency with the rest of the doc. **PR11613-F8 (Docs)** — Please add screenshots for the new Action controls, Submit Job panel, Checkpoints tab and Operations page, and refresh the existing ones so they reflect the new layout. Once those are in, I'll do one more pass. If any of the asks above are impractical for this PR, just say so and we can split them into follow-ups. <!-- streview-comment:2049 --> -- 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]
