DanielLeens commented on issue #12630:
URL: https://github.com/apache/seatunnel/issues/12630#issuecomment-5994184731
Classification: D / seatunnel-cli Zeta operational-diagnostics contract.
The model-free, one-request direction is useful, and #12631 already has the
right high-level separation: deterministic rules over engine-published fields,
no submission/stop/savepoint action, and a non-zero shell result only for a
terminal `FAILED` job. I checked the current engine source as well as the CLI
patch. There are three items to close before treating the implementation as
ready:
1. `JobInfoService.getJobInfoJson` returns HTTP 200 with only `{"jobId":
...}` when neither the running nor finished-job maps contain the id; it does
not return 404. Therefore the current #12631 404 test does not exercise the
production missing-job path, and the command would print `UNKNOWN` then exit 0.
Please recognize the minimal 200 response as "no retained job", return a
non-zero result, and add a loopback test for that exact payload.
2. `/job-info` is not a lightweight diagnostics read today.
`BaseService.convertToJson` restores the logical DAG, and a cache miss can fall
through to `DAGUtils.getJobDAGInfo(..., true, ...)`, which the source itself
marks as potentially slow and nondeterministic. The CLI only needs status,
diagnostics, counters, and an error message. Either document that the first
version deliberately pays for the existing full `job-info` path, or propose a
bounded lightweight diagnostics response before promising that this is
operationally harmless on a busy cluster.
3. Keep the REST-client extraction in #12629 rather than duplicating it in
this feature PR, and redact `RestError.body` before displaying it. That body is
opaque server text and currently bypasses `redact_credentials`; add a non-404
error-response regression containing a password and assert the secret is absent
while the error code remains visible.
Once those points are addressed, the rule and exit-code tests in #12631
provide a good base for reviewing the actual diagnosis behavior.
--
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]
