junrao commented on code in PR #19167:
URL: https://github.com/apache/kafka/pull/19167#discussion_r2019483491
##########
clients/src/main/java/org/apache/kafka/common/requests/FetchResponse.java:
##########
@@ -258,6 +258,11 @@ private static FetchResponseData toMessage(Errors error,
FetchResponseData.PartitionData partitionData = entry.getValue();
// Since PartitionData alone doesn't know the partition ID, we set
it here
partitionData.setPartitionIndex(entry.getKey().topicPartition().partition());
+ // To protect the clients from failing due to null records,
Review Comment:
I was thinking of the following. Make the following constructor private and
change FetchRequest.getErrorResponse() to use the `of` constructor.
` public FetchResponse(FetchResponseData fetchResponseData)`
Add a comment that the `of` constructor is for the server and the `parse`
constructor is for the client.
@chia7712 : Is the following TODO still valid?
```
// TODO: remove as a part of KAFKA-12410
public static FetchResponse of(Errors error,
```
##########
clients/src/main/java/org/apache/kafka/common/requests/FetchResponse.java:
##########
@@ -89,7 +89,7 @@ public FetchResponseData data() {
*/
public FetchResponse(FetchResponseData fetchResponseData) {
super(ApiKeys.FETCH);
- this.data = fetchResponseData;
+ this.data = convertNullRecordsToEmpty(fetchResponseData);
Review Comment:
Those partitions are created using the `ShareFetchResponse.of()`
constructor. If we initialize the records in the constructor properly, we don't
need to change it here.
--
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
