showuon commented on a change in pull request #11843:
URL: https://github.com/apache/kafka/pull/11843#discussion_r819220063
##########
File path:
clients/src/main/java/org/apache/kafka/clients/FetchSessionHandler.java
##########
@@ -454,7 +454,7 @@ String
verifyFullFetchResponsePartitions(Set<TopicPartition> topicPartitions, Se
* @param topicPartitions The topicPartitions from the FetchResponse.
* @param ids The topic IDs from the FetchResponse.
* @param version The version of the FetchResponse.
- * @return True if the incremental fetch response
partitions are valid.
+ * @return null if the incremental fetch response
partitions are valid; human-readable problem description otherwise.
Review comment:
Thanks for the nice catch! I'm wondering if the L467 and L469 are wrong.
Currently, we did:
```java
if (extra.isEmpty())
bld.append("extraPartitions=(").append(Utils.join(extra, ",
")).append("), ");
if (extraIds.isEmpty())
bld.append("extraIds=(").append(Utils.join(extraIds, ",
")).append("), ");
```
And that will only append **empty** extras and extraIds, which is not right.
Do you think it should be non-empty check?
##########
File path:
clients/src/main/java/org/apache/kafka/clients/FetchSessionHandler.java
##########
@@ -420,7 +420,7 @@ private String
topicIdPartitionsToLogString(Collection<TopicIdPartition> partiti
* @param topicPartitions The topicPartitions from the FetchResponse.
* @param ids The topic IDs from the FetchResponse.
* @param version The version of the FetchResponse.
- * @return True if the full fetch response partitions are
valid.
+ * @return null if the fetch response partitions are
valid; human-readable problem description otherwise.
Review comment:
I would keep the `full` word as before. Any reason why we should remove
it?
--
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]
