SEZ9 commented on PR #11557: URL: https://github.com/apache/seatunnel/pull/11557#issuecomment-5691200403
Thanks for the follow-up on PR11557-F1 in `96e52dff`. Switching `ManagedSourceRegisterOperation#readInternal` from the untyped `in.readObject()` to `in.readObject(TaskLocation.class)` is the right direction, and I agree it matches how `TaskOperation#readInternal` already handles its base `taskLocation` field, so this doesn't introduce a new pattern. Keeping the change scoped to this file and leaving the pre-existing sibling operations for a separate effort is fine with me for this PR. One thing I'd still like closed out before marking F1 resolved: the type-constrained overload only helps if Hazelcast actually enforces the `Class` argument on the `SerializerAdapter.read(ObjectDataInput, Class)` path used for `TaskLocation`, rather than treating it as a hint. I don't see the conclusion of that runtime check recorded yet, so could you either: 1. Add a short unit test that feeds `ManagedSourceRegisterOperation#readInternal` a payload whose `readerLocation` encodes a type other than `TaskLocation` and asserts that decoding fails rather than yielding an unexpected object, or 2. Post the concrete finding of that runtime check so it's on record. If the constrained overload turns out not to reject a mismatched type, we should discuss a defensive class check after the read instead. Otherwise, with the test or confirmation in place, I'm happy to consider F1 done for the delta between `2dc5c3de085f` and `96e52dff9c4`. <!-- streview-comment:1077 --> -- 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]
