SEZ9 commented on PR #12619: URL: https://github.com/apache/seatunnel/pull/12619#issuecomment-6229023243
Agreed on keeping `JobLineageException` as a plain local exception in this PR: since `ExceptionHandlingFilter` only maps `IllegalArgumentException` to 400 and everything else to 500, subclassing `SeaTunnelRuntimeException` would not give us 409/413 on its own and would prepend an error-code prefix to messages that are intentionally fixed per reason. Doing the explicit `Reason` → HTTP status mapping (and stable error codes, if wanted) in the endpoint PR is the right place for it. To close this out on my side, two small asks: 1. Add a short Javadoc note on `JobLineageException` stating that callers (the future servlet) are expected to map `Reason` to an HTTP status explicitly, so `LINEAGE_UNAVAILABLE` / `LINEAGE_GRAPH_TOO_LARGE` are not accidentally surfaced as 500 once `project()` is wired to a servlet. 2. Confirm whether the other points from the previous review (retained-bytes guard timing in `projectNodes`, the non-positive `jobId` rejection, the all-null `TablePath` case, the `REPORTED` + `DATASET_METADATA_UNAVAILABLE` contradiction, the null `limits` check, the `JobLineageLimits.DEFAULT` documentation, and the transform/logical-DAG test coverage) are addressed in the latest push or are intentionally deferred — a quick checklist in the PR description is enough. <!-- streview-comment:1820 --> -- 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]
