SEZ9 commented on PR #11557: URL: https://github.com/apache/seatunnel/pull/11557#issuecomment-6553045624
Thanks for the push to `41b5818d6b8` and for the detailed conflict table for the `dev` (`3ac2d1cec6a`) merge in `aa403302293`. Keeping dev's logic alongside this PR's managed-source runtime reads well to me, and moving the managed serializer ids in `TaskDataSerializerHook` is fine since they were never released. On F1 (`ManagedSourceRegisterOperation` deserialization), the update covers the merge, but I can't tell from it whether the three open items from my previous comment are addressed on this head. Could you confirm: 1. `SourceAssignmentTrackerTest.java:74` now declares `Collection<SourceAssignmentTracker.Entry>`, so `seatunnel-engine-server` compiles and its tests actually execute (they did not on `5dc655e6a91`). 2. The legacy wire-type validation link (`AssignSplitOperation#readInternal` / `TaskOperation#readInternal`) has been added to the PR description. 3. Once the Build on `41b5818d6b8` reaches `seatunnel-engine-server`, please post the per-test result for `ManagedSourceRegisterOperationSerializationTest` (both test names, with the job link). Since the serializer ids shifted in `TaskDataSerializerHook`, the run on the new head is the evidence that matters. F1 stays open until item 3 is posted. Everything else in the merge resolution looks good. <!-- streview-comment:2193 --> -- 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]
