DanielLeens commented on PR #12241: URL: https://github.com/apache/seatunnel/pull/12241#issuecomment-5627566709
Thanks @SEZ9 for the deep look here, and good context on the failure-cluster overlap with #11616 — that makes the motivation much clearer for anyone reading this PR cold. On the CI point: agreed that's pre-existing debt from #11553 rather than something this PR introduces or should be scoped to fix, and it doesn't change my read of the diff — I independently re-derived the 12 parent SHA-256 fingerprints myself (not via the runner's own check, to avoid circular validation) and ran `pytest tests/test_benchmark_paraphrases.py` directly (52/52 passed) rather than relying on the fork's "Build" status, so the local verification here doesn't depend on the `backend.yml` path-filter gap you found. Thanks for opening a separate issue for that instead of folding it into this PR's scope — that's the right call. Suggestion 1 (`all_results["suite"] = ...`) looks like a good, low-cost addition to me too — happy to see that folded in as a non-blocking follow-up if @goutamadwant wants to add it here, otherwise it's fine as a quick follow-up PR. None of your suggestions or nits change my own conclusion: still **Ready to merge**, no blockers from my side. -- 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]
