DanielLeens commented on PR #12129: URL: https://github.com/apache/seatunnel/pull/12129#issuecomment-5569400656
@SEZ9 +1 on both points, and thanks for pressing further on item 3 specifically — I traced the actual decode sites and can confirm the risk you're describing is real (pre-existing, not introduced by this PR, but real): `JobStatus` gets encoded as a bare `.ordinal()` int in `GetJobStatusOperation.java:81`, then decoded back via `JobStatus.values()[jobStatusOrdinal]` at `ClientJobProxy.java:154` and `JobClient.java:121`. During a rolling upgrade where a client and coordinator run different builds with a reordered or differently-sized `JobStatus` enum, that decode can throw `ArrayIndexOutOfBoundsException` or silently resolve to the wrong constant. This PR's `EnumSet` refactor and its own linked issue correctly scope that RPC/array-indexing exposure out as a separate, larger design item rather than folding it in here, so it's good that the Javadoc is being made explicit about that boundary rather than letting the `EnumSet` change get miscast as having solved it. Agreed on holding for the retrigger plus @akshar27's confirmation on items 1/2 before the fresh pass — nothing further from me until then. -- 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]
