SEZ9 commented on PR #11982:
URL: https://github.com/apache/seatunnel/pull/11982#issuecomment-5433982726
Thanks for the review — all three points were valid, and all three are
addressed.
**Code style (blocking).** Real failure, not a false positive. Fixed exactly
the hunks the job
reported: the `GetJobDiagnosticsOperation` import was out of order in
`ClientToServerOperationDataSerializerHook`, five statements in
`JobRuntimeDiagnosticsTest` were
hand-wrapped although they fit in 100 columns, and one assertion in
`RestApiIT` was 102 columns.
There is no JDK/Maven in my environment, so I could not run `mvn
spotless:apply`; the changes were
applied by hand against google-java-format 1.7/AOSP rules and every touched
file is now verified to
have no line over 100 columns and no wrap that google-java-format would
collapse. CI is the check
that matters here, so if the Code style job still flags anything I will
iterate on its diff.
**`getJobDiagnostics` master assumption.** The check is not missing, but it
was implicit: the caller
resolves the server with `getSeaTunnelServer(true)`, which returns `null` on
a non-master node, so
the argument is either `null` or this node is the master. Added a comment
stating that invariant so
the next reader does not have to re-derive it.
**`/running-jobs` per-job RPC.** Agreed, and rather than deferring it I
removed the cost:
`convertToJson` now takes a `withDiagnostics` flag and only the single-job
`job-info/{jobId}`
endpoint asks for diagnostics. The listing was paying one extra master round
trip *per running job*
for a field its callers do not use. Both REST planes share
`getRunningJobsJson`, so the v1 and v2
listings are both covered, and the v1/v2 docs now say which endpoint returns
the field. That keeps
this PR to the scope of #11980 Phase 1 (single-job diagnostics) and leaves a
batched/opt-in listing
variant as a separate change if anyone actually wants 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]
