SEZ9 commented on PR #11557: URL: https://github.com/apache/seatunnel/pull/11557#issuecomment-6051259065
Thanks for the detailed follow-up on F1 in `5a0f93b37de`. **Wire-type check.** The approach reads right to me: decoding via `in.readObject()` and rejecting anything that is not a `TaskLocation` (including `null`) with an explicit `IOException` at the read site addresses the override-not-validate problem, and leaning on the public `ObjectDataInput` contract rather than the internal header layout is the more robust choice. I'm fine with this resolving the first ask once it is shown green. **Unit test.** `testMismatchedReaderLocationTypeIsRejected` using a `Progress` in the reader-location slot (same factory id, different class id) is exactly the negative case I wanted, and the byte-for-byte round-trip is a good guard. However, since run `37345594530` executed no tests and you have not run them locally either, this is still unverified. Remaining asks: 1. Please run `ManagedSourceRegisterOperationSerializationTest` locally and paste the result here, so we are not blocked solely on the CI rerun. Specifically, please also confirm that `testMismatchedReaderLocationTypeIsRejected` fails when run against the previous `readObject(TaskLocation.class)` implementation, as you described — that is the key evidence the test actually exercises the fix. 2. Once the CI blocker is cleared, re-run `5a0f93b37de` and link the green run on this thread. **Other `readObject()` calls.** Agreed on scope: keeping `AssignSplitOperation#readInternal` (`taskID = in.readObject()`) and the base `TaskOperation#readInternal` out of this PR is reasonable since both are pre-existing on `dev` and shared beyond the managed lane. Please file the tracking issue and link it here before we merge; I'll treat the second F1 ask as open until that link is posted. No further changes needed on F1 beyond the verification and the tracking issue above. <!-- streview-comment:1594 --> -- 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]
