frankvicky commented on code in PR #19519:
URL: https://github.com/apache/kafka/pull/19519#discussion_r2055664719
##########
clients/src/test/java/org/apache/kafka/common/metrics/internals/PluginMetricsImplTest.java:
##########
@@ -36,11 +35,15 @@
public class PluginMetricsImplTest {
- private final Map<String, String> extraTags =
Collections.singletonMap("my-tag", "my-value");
+ private static final LinkedHashMap<String, String> EXTRA_TAGS = new
LinkedHashMap<>();
Review Comment:
Could we eliminate the static keyword?
`private final LinkedHashMap<String, String> EXTRA_TAGS = new
LinkedHashMap<>(Map.of("my-tag", "my-value"));`
##########
clients/src/test/java/org/apache/kafka/clients/producer/KafkaProducerTest.java:
##########
@@ -412,13 +401,18 @@ public void
testRetriesAndIdempotenceForIdempotentProducers() {
"Must set retries to non-zero when using the transactional
producer.");
}
- @Test
- public void testInflightRequestsAndIdempotenceForIdempotentProducers() {
+ private static Properties baseProperties() {
Properties baseProps = new Properties() {{
setProperty(ProducerConfig.BOOTSTRAP_SERVERS_CONFIG,
"localhost:9999");
setProperty(ProducerConfig.KEY_SERIALIZER_CLASS_CONFIG,
StringSerializer.class.getName());
setProperty(ProducerConfig.VALUE_SERIALIZER_CLASS_CONFIG,
StringSerializer.class.getName());
}};
Review Comment:
Could we also clean this, and do we need static here?
##########
server/src/test/java/org/apache/kafka/security/authorizer/AuthorizerUtilsTest.java:
##########
@@ -76,23 +77,27 @@ public void testCreateAuthorizer(ProcessRole role) throws
ClassNotFoundException
assertEquals(ServerConfigs.AUTHORIZER_CLASS_NAME_CONFIG,
metricName.tags().get("config"));
assertEquals(MonitorableAuthorizer.class.getSimpleName(),
metricName.tags().get("class"));
assertEquals(role.toString(), metricName.tags().get("role"));
-
assertTrue(metricName.tags().entrySet().containsAll(monitorableAuthorizer.extraTags.entrySet()));
+
assertTrue(metricName.tags().entrySet().containsAll(MonitorableAuthorizer.EXTRA_TAGS.entrySet()));
assertEquals(0, metrics.metric(metricName).metricValue());
monitorableAuthorizer.authorize(null, null);
assertEquals(1, metrics.metric(metricName).metricValue());
}
public static class MonitorableAuthorizer implements Authorizer,
Monitorable {
- private final Map<String, String> extraTags = Map.of("k1", "v1");
+ private static final LinkedHashMap<String, String> EXTRA_TAGS = new
LinkedHashMap<>();
Review Comment:
ditto
##########
connect/runtime/src/test/java/org/apache/kafka/connect/runtime/ConnectMetricsTest.java:
##########
@@ -64,10 +64,13 @@ public class ConnectMetricsTest {
WorkerConfig.KEY_CONVERTER_CLASS_CONFIG,
"org.apache.kafka.connect.json.JsonConverter",
WorkerConfig.VALUE_CONVERTER_CLASS_CONFIG,
"org.apache.kafka.connect.json.JsonConverter");
private static final ConnectorTaskId CONNECTOR_TASK_ID = new
ConnectorTaskId("connector", 0);
- private static final Map<String, String> TAGS = Map.of("t1", "v1");
-
+ private static final LinkedHashMap<String, String> TAGS = new
LinkedHashMap<>();
Review Comment:
ditto
##########
clients/src/test/java/org/apache/kafka/clients/producer/KafkaProducerTest.java:
##########
@@ -2881,9 +2875,13 @@ private MetricName expectedMetricName(String clientId,
String config, Class<?> c
private static final String NAME = "name";
private static final String DESCRIPTION = "description";
- private static final Map<String, String> TAGS =
Collections.singletonMap("k", "v");
+ private static final LinkedHashMap<String, String> TAGS = new
LinkedHashMap<>();
Review Comment:
`private static final LinkedHashMap<String, String> TAGS = new
LinkedHashMap<>(Map.of("t1", "v1"));`
--
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: jira-unsubscr...@kafka.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
