SEZ9 commented on PR #12173: URL: https://github.com/apache/seatunnel/pull/12173#issuecomment-5650138833
@Rangsh — thanks for finishing the F6 write-up. Agreed: `JobHistoryServiceFinishedMetricsTest` in `ca82f95` now asserts the observable contract (single `put(jobId, …, ttl, MINUTES)` with the merged value plus `lock(jobId)` / `unlock(jobId)`), covers the null-metrics rejection via `storeFinishedPipelineMetricsRejectsNullMetrics`, and verifies the TTL-preserving put path. F6 is resolved from my side, and my verify pass over `c553a8c04` / `ca82f95` / `9a6077c58` raises no new review points. One blocker remains before this head can go green, as noted above: `Run / benchmark-test` fails on both JDK 8 and JDK 11 (fork run https://github.com/Rangsh/seatunnel/actions/runs/34558258789) because `spotless:check` rejects the new comment line added in `c553a8c04` to `IMapJobGrowthBenchmarkWorkload.java`. It's deterministic, so it needs a new push — `mvn spotless:apply -pl seatunnel-benchmarks` should fix it directly. The other failing connector IT jobs on that run don't touch this PR's diff, but please re-confirm they're green after CI reruns on the new head. Once the spotless fix is pushed and `Build` is green, nothing else is open from my side. I only have comment-only review rights here, so formal approval and merge will need to come from a committer. <!-- streview-comment:998 --> -- 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]
