m1a2st commented on code in PR #19372:
URL: https://github.com/apache/kafka/pull/19372#discussion_r2029065027
##########
core/src/main/java/kafka/server/share/SharePartitionManager.java:
##########
@@ -357,7 +356,7 @@ public CompletableFuture<Map<TopicIdPartition,
ShareAcknowledgeResponseData.Part
Review Comment:
Could you also update L354 `log.debug("Removed share session with key {}",
key);`
##########
core/src/test/java/kafka/security/JaasModule.java:
##########
@@ -62,7 +61,7 @@ public static JaasModule oAuthBearerLoginModule(String
username, boolean debug)
}
public static JaasModule plainLoginModule(String username, String
password) {
Review Comment:
This method is unused, we can remove it
##########
core/src/main/java/kafka/server/share/SharePartition.java:
##########
@@ -63,7 +63,6 @@
import org.slf4j.LoggerFactory;
import java.util.ArrayList;
-import java.util.Collections;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
Review Comment:
Could you remove L2451 unused throw `exception`
##########
core/src/test/java/kafka/server/integration/EligibleLeaderReplicasIntegrationTest.java:
##########
@@ -349,7 +348,7 @@ public void
testElrMemberShouldBeKickOutWhenUncleanShutdown(String quorum) throw
waitForIsrAndElr((isrSize, elrSize) -> {
return isrSize == 0 && elrSize == 2;
});
- topicPartitionInfo =
adminClient.describeTopics(Collections.singletonList(testTopicName))
+ topicPartitionInfo =
adminClient.describeTopics(List.of(testTopicName))
.allTopicNames().get().get(testTopicName).partitions().get(0);
assertTrue(topicPartitionInfo.leader() == null);
Review Comment:
`assertTrue(topicPartitionInfo.leader() == null)` to `assertNull`
##########
core/src/test/java/kafka/security/JaasTestUtils.java:
##########
@@ -246,7 +245,7 @@ public static JaasSection
kafkaClientSection(Optional<String> mechanism, Optiona
KAFKA_SCRAM_PASSWORD_2,
KAFKA_OAUTH_BEARER_USER_2,
SERVICE_NAME)
-
).map(Collections::singletonList).orElse(Collections.emptyList()));
+ ).map(List::of).orElse(List.of()));
}
private static void writeToFile(File file, List<JaasSection> jaasSections)
throws IOException {
Review Comment:
Could you also update L258, L268 switch case?
##########
core/src/test/java/kafka/server/integration/EligibleLeaderReplicasIntegrationTest.java:
##########
@@ -415,7 +414,7 @@ public void
testLastKnownLeaderShouldBeElectedIfEmptyElr(String quorum) throws E
waitForIsrAndElr((isrSize, elrSize) -> {
return isrSize == 0 && elrSize == 1;
});
- topicPartitionInfo =
adminClient.describeTopics(Collections.singletonList(testTopicName))
+ topicPartitionInfo =
adminClient.describeTopics(List.of(testTopicName))
.allTopicNames().get().get(testTopicName).partitions().get(0);
assertTrue(topicPartitionInfo.leader() == null);
Review Comment:
`assertTrue(topicPartitionInfo.leader() == null)` to `assertNull`
##########
core/src/test/java/kafka/server/handlers/DescribeTopicPartitionsRequestHandlerTest.java:
##########
@@ -463,7 +461,7 @@ void testDescribeTopicPartitionsRequestWithEdgeCases() {
// 3.4 With cursor point to a negative partition id. Exception should
be thrown if not querying all the topics.
describeTopicPartitionsRequest = new
DescribeTopicPartitionsRequest(new DescribeTopicPartitionsRequestData()
- .setTopics(Arrays.asList(
+ .setTopics(List.of(
new
DescribeTopicPartitionsRequestData.TopicRequest().setName(authorizedTopic),
new
DescribeTopicPartitionsRequestData.TopicRequest().setName(authorizedTopic2)
Review Comment:
Please also update L493 `List.stream().foreach()` to `List.foreach`
##########
core/src/test/java/kafka/server/integration/EligibleLeaderReplicasIntegrationTest.java:
##########
@@ -415,7 +414,7 @@ public void
testLastKnownLeaderShouldBeElectedIfEmptyElr(String quorum) throws E
waitForIsrAndElr((isrSize, elrSize) -> {
Review Comment:
L411 `List.stream().foreach()` to `List.foreach`
##########
core/src/test/java/kafka/server/share/SharePartitionManagerTest.java:
##########
@@ -592,19 +591,19 @@ public void testToForgetPartitions() {
Review Comment:
Just curiously, L163 `Collections.unmodifiableList(new ArrayList<>());` can
change to `List.of()`?
##########
core/src/test/java/kafka/server/integration/EligibleLeaderReplicasIntegrationTest.java:
##########
@@ -261,7 +260,7 @@ public void testElrMemberCanBeElected(String quorum) throws
ExecutionException,
return isrSize == 0 && elrSize == 3;
});
- topicPartitionInfo =
adminClient.describeTopics(Collections.singletonList(testTopicName))
+ topicPartitionInfo =
adminClient.describeTopics(List.of(testTopicName))
.allTopicNames().get().get(testTopicName).partitions().get(0);
assertEquals(1, topicPartitionInfo.lastKnownElr().size(),
topicPartitionInfo.toString());
int expectLastKnownLeader = initialReplicas.get(3).id();
Review Comment:
L273 `.collect(Collectors.toList())` to `.toList()`
--
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
