SEZ9 commented on PR #11841: URL: https://github.com/apache/seatunnel/pull/11841#issuecomment-5975902512
Thanks for the follow-up in `af98c93c064`. Rebuilding `restoreMode_shouldMatchLegacySavepointFlag` through the real `JobImmutableInformation` constructor, asserting `isSavepointRestore()` / `isRestoreJob()` for every `RestoreMode` value and across the `writeData`/`readData` round trip, and adding `shouldReadLegacyPayloadWithoutSavepoint` closes the test-invariant gap I raised earlier. My approval stands. One small non-blocking item left from my last review: "Does this PR introduce any user-facing change?" and "How was this patch tested?" are still blank in the PR body. For the user-facing section, please note that checkpoint-mode restore submissions which hit a pending cleanup record or existing job metrics now fail with `JobException` rather than following the savepoint path, since that is an intentional behavior change on upgrade worth surfacing in release notes. For the testing section, a short list of the compatibility tests you added or updated is enough. @dybyte thanks for the +1 — agreed, a green CI run on this head and then this is good to go. <!-- streview-comment:1501 --> -- 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]
