SEZ9 commented on PR #12134: URL: https://github.com/apache/seatunnel/pull/12134#issuecomment-6008891978
Thanks for the update. The redrive fix in `CoordinatorService`/`JobMaster` is unchanged since the last round, so I'm not reopening the earlier points. The only new change is `872701ec8`, which widens the assertion in `testStreamJobFailsAfterCheckpointTriggerDispatchFailure` to accept either `CheckpointCloseReason.CHECKPOINT_INSIDE_ERROR` or `CHECKPOINT_NOTIFY_COMPLETE_FAILED`. Both values are reachable from the same injected fault through `CheckpointManager#sendOperationToMemberNode`, so the widening itself looks correct to me. Since that test method came into this branch via a `dev` merge rather than being part of this PR, could you add a short note (in the PR description or commit message) explaining why the assertion needed widening here, e.g. whether it was flaking on this branch? That will make the change easier to follow for anyone reading the history later. Happy to take a final pass once that's in. <!-- streview-comment:1552 --> -- 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]
