anatasiavela commented on a change in pull request #9526:
URL: https://github.com/apache/kafka/pull/9526#discussion_r517665383
##########
File path:
generator/src/main/java/org/apache/kafka/message/JsonConverterGenerator.java
##########
@@ -380,8 +380,9 @@ private void generateVariableLengthTargetToJson(Target
target, Versions versions
target.sourceVariable(), target.sourceVariable())));
}
} else if (target.field().type().isRecords()) {
- headerGenerator.addImport(MessageGenerator.BINARY_NODE_CLASS);
- buffer.printf("%s;%n", target.assignmentStatement("new
BinaryNode(new byte[]{})"));
+ headerGenerator.addImport(MessageGenerator.INT_NODE_CLASS);
+ buffer.printf("%s;%n", target.assignmentStatement(
+ String.format("new IntNode(%s.sizeInBytes())",
target.sourceVariable())));
Review comment:
@lbradstreet The current generated JSON does not print the recordSet.
When we were serializing a BinaryNode with an empty array, it was still being
deserialized as a BinaryNode, so it doesn't break anything.
```
buffer.printf("%s;%n", target.assignmentStatement(
String.format("MemoryRecords.readableRecords(ByteBuffer.wrap(MessageUtil.jsonNodeToBinary(%s,
\"%s\")))",
target.sourceVariable(), target.humanReadableName())));
```
So I believe the concern is that I had proposed to change the serialization
to an IntNode, but the deserialization still expects a BinaryNode which I
should have also changed.
Another alternative I could fix this by changing the deserialization to
expect an IntNode, but I'm for adding a verbose flag
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
