ijuma commented on code in PR #18726:
URL: https://github.com/apache/kafka/pull/18726#discussion_r1976186661
##########
clients/src/main/resources/common/message/FetchResponse.json:
##########
@@ -106,7 +106,7 @@
]},
{ "name": "PreferredReadReplica", "type": "int32", "versions": "11+",
"default": "-1", "ignorable": false, "entityType": "brokerId",
"about": "The preferred read replica for the consumer to use on its
next fetch request."},
- { "name": "Records", "type": "records", "versions": "0+",
"nullableVersions": "0+", "about": "The record data."}
Review Comment:
@junrao I don't think we can break so many clients in the wild. That would
definitely require a KIP (and I would be against a KIP that did that
immediately - it would require years of notice). So, we'd have to specify very
clearly when null can or cannot be set. Your proposed definition doesn't
actually cover the case that breaks librdkafka: [authz
errors](https://github.com/confluentinc/librdkafka/blob/876cf4a7e4a339fda8cea1c4a650c168a351a176/tests/0119-consumer_auth.cpp#L141).
My take:
1. I can remove non-nullable from the schema if people feel strongly.
2. But I will still make sure the Kafka code never returns null for records.
It's actually more work to ensure the latter without the former, but I am ok
going that direction. I am not ok with any solution that breaks recently
released and widely deployed clients.
--
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]
