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 either. But when we are serializing a BinaryNode with an empty array, it is 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