dajac commented on code in PR #13511:
URL: https://github.com/apache/kafka/pull/13511#discussion_r1163731655
##########
clients/src/main/java/org/apache/kafka/common/protocol/MessageUtil.java:
##########
@@ -231,4 +231,38 @@ public static byte[] toVersionPrefixedBytes(final short
version, final Message m
buffer.limit() == buffer.array().length) return buffer.array();
else return Utils.toArray(buffer);
}
+
+ // Should only be used for testing
+ public static byte[] messageWithUnknownVersion() {
Review Comment:
It is a bit awkward to have this in the `src` directory. I am not really
sure to understand why we need it as well. I suppose that we could use any
other `Message` like record from `test`, isn't it?
##########
core/src/main/scala/kafka/coordinator/group/GroupMetadataManager.scala:
##########
@@ -1145,17 +1150,22 @@ object GroupMetadataManager {
* @param buffer input byte-buffer
* @return an OffsetKey or GroupMetadataKey object from the message
*/
- def readMessageKey(buffer: ByteBuffer): BaseKey = {
+ def readMessageKey(buffer: ByteBuffer): Option[BaseKey] = {
Review Comment:
Have you considered introducing an `UnknownKey` which extends `BaseKey`? The
`UnknownKey` could carry on the version and even the bytes. It seems that the
change would be less disruptive like this.
--
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
