You are viewing a plain text version of this content. The canonical link for it is here.
Posted to jira@kafka.apache.org by "FrankYang0529 (via GitHub)" <gi...@apache.org> on 2026/03/30 11:45:45 UTC
[PR] KAFKA-20371: Add Iterable constructor to generated message collections [kafka]
FrankYang0529 opened a new pull request, #21894:
URL: https://github.com/apache/kafka/pull/21894
The new constructor will enable us to use `List.of()` instead of `List.of().iterator()`. This new constructor will simply extract the iterator and pass it to the parent class.
--
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-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-20371: Add Iterable constructor to generated message collections [kafka]
Posted by "FrankYang0529 (via GitHub)" <gi...@apache.org>.
FrankYang0529 commented on code in PR #21894:
URL: https://github.com/apache/kafka/pull/21894#discussion_r3021533816
##########
clients/src/main/java/org/apache/kafka/common/utils/internals/ImplicitLinkedHashCollection.java:
##########
@@ -561,6 +561,17 @@ public ImplicitLinkedHashCollection(Iterator<E> iter) {
}
}
+ /**
+ * Create a new ImplicitLinkedHashCollection.
+ *
+ * @param iterable We will add all the elements accessible through this iterable
+ * to the set.
+ */
+ @SuppressWarnings("this-escape")
+ public ImplicitLinkedHashCollection(Iterable<E> iterable) {
Review Comment:
Yes, thanks for the reminder.
--
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-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-20371: Add Iterable constructor to generated message collections [kafka]
Posted by "chia7712 (via GitHub)" <gi...@apache.org>.
chia7712 commented on code in PR #21894:
URL: https://github.com/apache/kafka/pull/21894#discussion_r3016159260
##########
clients/src/main/java/org/apache/kafka/common/utils/internals/ImplicitLinkedHashCollection.java:
##########
@@ -561,6 +561,17 @@ public ImplicitLinkedHashCollection(Iterator<E> iter) {
}
}
+ /**
+ * Create a new ImplicitLinkedHashCollection.
+ *
+ * @param iterable We will add all the elements accessible through this iterable
+ * to the set.
+ */
+ @SuppressWarnings("this-escape")
+ public ImplicitLinkedHashCollection(Iterable<E> iterable) {
Review Comment:
The generated code can use `Iterable.iterator`, right?
--
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-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-20371: Add Iterable constructor to generated message collections [kafka]
Posted by "FrankYang0529 (via GitHub)" <gi...@apache.org>.
FrankYang0529 merged PR #21894:
URL: https://github.com/apache/kafka/pull/21894
--
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-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-20371: Add Iterable constructor to generated message collections [kafka]
Posted by "clolov (via GitHub)" <gi...@apache.org>.
clolov commented on code in PR #21894:
URL: https://github.com/apache/kafka/pull/21894#discussion_r3021641265
##########
metadata/src/test/java/org/apache/kafka/controller/QuorumControllerTest.java:
##########
@@ -1124,7 +1124,7 @@ public void testSnapshotSaveAndLoad() throws Throwable {
new CreatableReplicaAssignment().
setPartitionIndex(1).
setBrokerIds(List.of(1, 2, 0))).
- iterator()))).iterator())),
+ iterator()))))),
Review Comment:
Nit: This iterator seems to be removable as well 😊
--
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-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-20371: Add Iterable constructor to generated message collections [kafka]
Posted by "FrankYang0529 (via GitHub)" <gi...@apache.org>.
FrankYang0529 commented on PR #21894:
URL: https://github.com/apache/kafka/pull/21894#issuecomment-4177648129
@clolov Thanks for the suggestion. Updated it.
> I removed the `generateHashSetIteratorConstructor` method and while there are some genuine uses of Iterator, like VoterSet#toVotersRecord, there are also quite a lot of other usages where one could remove the iterator() call, is this something you were planning on doing next or you had already thought about it and rejected it for a reason?
We don't have answer yet, but I think we can remove `generateHashSetIteratorConstructor` method. @chia7712 WDYT?
--
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-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-20371: Add Iterable constructor to generated message collections [kafka]
Posted by "clolov (via GitHub)" <gi...@apache.org>.
clolov commented on PR #21894:
URL: https://github.com/apache/kafka/pull/21894#issuecomment-4177540153
Thanks for the last changes! I think the last stragglers I found are
```
diff --git a/clients/src/test/java/org/apache/kafka/clients/admin/internals/DescribeTransactionsHandlerTest.java b/clients/src/test/java/org/apache/kafka/clients/admin/internals/DescribeTransactionsHandlerTest.java
index a2ec97868d..be60e89eb6 100644
--- a/clients/src/test/java/org/apache/kafka/clients/admin/internals/DescribeTransactionsHandlerTest.java
+++ b/clients/src/test/java/org/apache/kafka/clients/admin/internals/DescribeTransactionsHandlerTest.java
@@ -184,7 +184,7 @@ public class DescribeTransactionsHandlerTest {
new DescribeTransactionsResponseData.TopicData()
.setTopic("bar")
.setPartitions(asList(1, 3, 5))
- ).iterator()));
+ )));
}
private DescribeTransactionsResponseData.TransactionState sampleTransactionState2(
diff --git a/clients/src/test/java/org/apache/kafka/clients/consumer/internals/ConsumerProtocolTest.java b/clients/src/test/java/org/apache/kafka/clients/consumer/internals/ConsumerProtocolTest.java
index b73576757e..3f32a22b36 100644
--- a/clients/src/test/java/org/apache/kafka/clients/consumer/internals/ConsumerProtocolTest.java
+++ b/clients/src/test/java/org/apache/kafka/clients/consumer/internals/ConsumerProtocolTest.java
@@ -218,7 +218,7 @@ public class ConsumerProtocolTest {
new ConsumerProtocolAssignment.TopicPartition()
.setTopic(tp2.topic())
.setPartitions(Collections.singletonList(tp2.partition()))
- ).iterator())
+ ))
)
.setUserData(ByteBuffer.wrap("hello".getBytes()));
diff --git a/clients/src/test/java/org/apache/kafka/common/requests/DeleteGroupsRequestTest.java b/clients/src/test/java/org/apache/kafka/common/requests/DeleteGroupsRequestTest.java
index c6cd18a347..95c35ccaad 100644
--- a/clients/src/test/java/org/apache/kafka/common/requests/DeleteGroupsRequestTest.java
+++ b/clients/src/test/java/org/apache/kafka/common/requests/DeleteGroupsRequestTest.java
@@ -43,7 +43,7 @@ public class DeleteGroupsRequestTest {
new DeleteGroupsResponseData.DeletableGroupResult()
.setGroupId(groupId2)
.setErrorCode(Errors.COORDINATOR_LOAD_IN_PROGRESS.code())
- ).iterator());
+ ));
```
and I think we are good to merge 😊
--
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-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org