mjsax commented on code in PR #17182:
URL: https://github.com/apache/kafka/pull/17182#discussion_r1759616987
##########
streams/src/test/java/org/apache/kafka/streams/kstream/internals/KStreamSessionWindowAggregateProcessorTest.java:
##########
@@ -80,7 +80,7 @@ public class KStreamSessionWindowAggregateProcessorTest {
private final MockTime time = new MockTime();
private final Metrics metrics = new Metrics();
- private final StreamsMetricsImpl streamsMetrics = new
StreamsMetricsImpl(metrics, "test", StreamsConfig.METRICS_LATEST, time);
+ private final StreamsMetricsImpl streamsMetrics = new
StreamsMetricsImpl(metrics, "test", time);
Review Comment:
This is fine -- no need to pass into `StreamMetricImpl` any longer.
##########
streams/src/test/java/org/apache/kafka/streams/processor/internals/metrics/StreamsMetricsImplTest.java:
##########
@@ -139,7 +139,7 @@ public class StreamsMetricsImplTest {
private final MetricName metricName2 =
new MetricName(METRIC_NAME1, CLIENT_LEVEL_GROUP, DESCRIPTION2,
clientLevelTags);
private final MockTime time = new MockTime(0);
- private final StreamsMetricsImpl streamsMetrics = new
StreamsMetricsImpl(metrics, CLIENT_ID, VERSION, time);
Review Comment:
Where does `VERSION` come from? Can we delete the variable completely?
##########
streams/src/test/java/org/apache/kafka/streams/StreamsConfigTest.java:
##########
@@ -653,30 +653,6 @@ public void
shouldThrowExceptionIfNotAtLeastOnceOrExactlyOnce() {
assertThrows(ConfigException.class, () -> new StreamsConfig(props));
}
- @Test
Review Comment:
We should keep all these tests (cf. my other comment above).
We should also keep all text code below which set the config... (don't want
to comment on each file, so just make this single comment here)
##########
streams/src/main/java/org/apache/kafka/streams/StreamsConfig.java:
##########
@@ -496,10 +493,6 @@ public class StreamsConfig extends AbstractConfig {
public static final String BUFFERED_RECORDS_PER_PARTITION_CONFIG =
"buffered.records.per.partition";
public static final String BUFFERED_RECORDS_PER_PARTITION_DOC = "Maximum
number of records to buffer per partition.";
- /** {@code built.in.metrics.version} */
- public static final String BUILT_IN_METRICS_VERSION_CONFIG =
"built.in.metrics.version";
- private static final String BUILT_IN_METRICS_VERSION_DOC = "Version of the
built-in metrics to use.";
Review Comment:
We cannot make any changes to `StreamsConfig` as it would be a public API
change (which requires a KIP).
As pointed out on the Jira ticket, we can just keep it, to avoid the need to
do another KIP to re-introduce this parameter if we would need it again.
--
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]
