DanielLeens commented on PR #12130: URL: https://github.com/apache/seatunnel/pull/12130#issuecomment-5578499005
@SEZ9 @awsomesud347 Confirming I've re-checked the current head (`2dd45e26f8613e0ee682d48aed9e963fc1d004f7`) and it's exactly the commit my last review covered — nothing has moved since that approval. @SEZ9, I think our comments crossed: the checklist you re-posted is the same one from my second-round review, and my third pass (the one ending "Ready to merge, blockers: none") already confirms all seven items resolved at `2dd45e26f` — five by code change, the `start == total` boundary kept intentionally (matches legacy `writeJsonWithPagination` behavior, now documented), and the `matchingJobStates` cost recorded as a follow-up in the PR description. GitHub just collapses that review body behind "show more," which is presumably why it read as cut off. Good to hear your independent re-trace of the problem/fix matches mine — appreciate you double-checking the diff rather than taking my word for it. The one thing still open is a real green `Build` run: it's currently showing cancelled from `kudu-connector-it` hitting the 90-minute job budget twice, which per @awsomesud347's account is a timeout rather than a test failure, and unrelated to this diff (`PageBaseServlet`/`JobInfoService`/`FinishedJobsServlet`, no Kudu code path touched). That matches the same "unrelated connector E2E timeout" pattern as the Paimon/Cosmos DB flakes from the prior run. My approval stands, conditioned on getting that rerun clean the way I noted before — no further code changes needed on my end. Thanks both for the thorough back-and-forth here — this is a good example of review actually sharpening a PR rather than just rubber-stamping it. -- 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]
