[GitHub] [pulsar] sijie commented on issue #4318: Fixed the display of schema information

[GitBox <gi...@apache.org>]

sijie commented on issue #4318: Fixed the display of schema information
URL: https://github.com/apache/pulsar/pull/4318#issuecomment-494213891
 
 
   I am not in favor of using `String` for storing schema data. because String requires correct charsets to encode and decode. So I would suggest keeping `byte[]` for storing schema data.
   
   GetSchemaResponse made a bad assumption on converting `byte[]` to `string`. Ideally it should be encoded using base64 because json doesn't support `byte[]` well. It is a legal BC issue that we have to face. 
   
   that said, I don't think it is a good idea to use `GetSchemaResponse` directly. We should still convert the `GetSchemaResponse` to SchemaInfo. We should also consider fixing`GetSchemaResponse` to carry `byte[]` in base64 encoded format.
   
   We can have a util class on displaying `SchemaInfo` in a correct format.
   
   - For struct schema (avro, json, protobuf), we are using avro schema definition on defining the schema structure. So we can convert the `byte[]` back to the json string and display the json string in a pretty format.
   
   - For non-struct schema, we should just use base64 for displaying schema data. 
   
   In summary, my opinion is:
   
   - it is a display issue. We should just fix the display issue rather than changing the return result.
   - GetSchemaResponse is buggy. We should consider improving or fixing it.

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services