SEZ9 commented on PR #11597: URL: https://github.com/apache/seatunnel/pull/11597#issuecomment-5366241434
Thanks @DanielLeens for the thorough self-review at `20538ae1ebe78cc2d35871fbd02e1d79b960e154` — the re-traced call chain and the corrected write-ups are easy to follow. The feature looks reasonable to me: a purely additive, read-only per-job slot-usage endpoint fills a real gap, since `/overview` only exposes cluster-wide totals. A few things before I can approve: 1. **Tests** — I don't see test coverage mentioned in your write-ups. Does the diff include unit tests for the `RunningJobSlotUsageBuilder` aggregation logic and e2e coverage for both the v1 and v2 endpoints, including the non-master → master forwarding path? If not, please add them. 2. **Rebase** — your own compare notes the branch is `behind_by=28` relative to `dev`. Even though you found the ahead-commits don't overlap with the engine files, please rebase onto latest `dev` so CI runs against a current base. 3. **Response format** — the example serializes `jobId` (e.g. 733584788375093248) as a string. Please confirm that's intentional and consistent between the v1 and v2 responses. Once those are addressed I'm happy to do the formal approval, since self-approval is blocked on your account. <!-- streview-comment:417 --> -- 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]
