SEZ9 commented on PR #12173: URL: https://github.com/apache/seatunnel/pull/12173#issuecomment-6378537156
@Rangsh thanks for checking both points against `b137173f7` — you are right on both, and I am withdrawing them. 1. `never().computeIfAbsent`: I agree there is no `computeIfAbsent` reference anywhere in `JobHistoryServiceFinishedMetricsTest` at this head. The success paths assert on the observable contract — the TTL-preserving `put(eq(jobId), metricsCaptor.capture(), eq(1440L), eq(TimeUnit.MINUTES))` verifications at `:100-105`, `:121-126` and `:155-160` plus the captured stored metrics — and the null-metrics path is covered at `:135-138`. That is exactly what PR12173-F6 asked for, so F6 is closed on this head. 2. Test name: my mistake. The test at `:142` is `storeFinishedPipelineMetricsKeepsDistinctTaggedMeasurements`; `storeFinishedPipelineMetricsKeepsExistingMetricsForOtherTags` does not exist in this class, and the `:155-160` lines are the `put` verification inside `KeepsDistinctTaggedMeasurements`. I was quoting a stale name. No new push is needed for F6. The other items from my earlier review (the durability sample in `IMapJobGrowthBenchmarkWorkload`, the non-atomic get/merge/put and its Javadoc in `JobHistoryService`, and the `benchmark.md` methodology note) are unaffected by this correction and remain as previously discussed; I will keep follow-up on those in their own threads rather than mixing them in here. <!-- streview-comment:1979 --> -- 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]
