awsomesud347 commented on PR #12130: URL: https://github.com/apache/seatunnel/pull/12130#issuecomment-5578311990
@SEZ9 I posted a per-item response about 25 minutes before your comment, so you may not have seen it: https://github.com/apache/seatunnel/pull/12130#issuecomment-5578063614 All seven are addressed at `2dd45e26f`. Pointers, in your numbering: 1. `PageBaseServlet.pageParams()` plus the new `positiveIntParam()` helper. `rows` is validated like `page`, the offset is computed in `long` and rejected above `Integer.MAX_VALUE` before narrowing, and parse failures name the parameter. 2. `PageBaseServlet.checkPageInRange()`. Empty page, deliberately, and it matches the legacy `writeJsonWithPagination` condition (`start > total || page < 1`) byte for byte. There is a comment on the method recording why. 3. `PageBaseServlet.writeJsonWithPagination()` now routes through `pageParams()`, `checkPageInRange()` and `writeJsonPage()`, so the duplication is gone rather than documented. 4. `JobInfoService.getJobsByStateJson(state, start, rows)`, explicit checks for null `state`, negative `start` and `rows` below 1. 5. `docs/en/engines/zeta/rest-api-v2.md` and the `zh` equivalent, plus an `incompatible-changes.md` entry in both languages naming all three affected endpoints. 6. New `FinishedJobsServletTest`, nine tests covering the servlet surface and the wiring. 7. In the PR description. Also, DanielLeens's review is not truncated, GitHub just collapses long comments behind a "show more" control. His verdict at the end is "Ready to merge, blockers: none," with three Low severity nits. -- 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]
