DanielLeens commented on PR #12152: URL: https://github.com/apache/seatunnel/pull/12152#issuecomment-5726042299
@CryoThrust thanks for chasing that down rather than assuming — the `Field.set` on a `private static final` throwing `IllegalAccessException` on JDK 11 is a good catch, and it matches what I'd expect (the JVM blocks that particular reflective write by default, instance finals aside). Given that, I'd rather track it than reopen this diff for a third time. My reasoning from the Sep 16 review still holds: the double-rejection branch is a designed, intentional last-resort path, but it's currently unreachable in production (nothing shuts `ERROR_REPORT_FALLBACK_EXECUTOR` down anywhere in this codebase today), so it doesn't carry the same urgency as F1/F2 did — those were live, reachable hangs. Adding a `@VisibleForTesting` seam just to prove an unreachable branch isn't worth another round of churn on a diff the three of us have already picked apart twice. So: please file it as a follow-up alongside #12342 rather than pushing a new commit here. From my side the design is sound and I have nothing further blocking — my Sep 16 review already landed on "ready to merge once CI is green" with that item as a non-blocking recommendation, and today's re-check shows the same head (`44f762eac`) and the same `Build` failure pattern (connector-v2/transform-v2/rocketmq/engine-v2 integration lanes that don't touch `CheckpointCoordinator.java`), consistent with the fork-infra issue we've already traced across this PR and #12149/#12173/#12315. Once that clears (or is confirmed environmental against a fresh run), this is good to go from me. -- 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]
