ndimiduk commented on issue #754: HBASE-22978 : Online slow response log
URL: https://github.com/apache/hbase/pull/754#issuecomment-585986284
 
 
   > I have tried to cover these points in my recent commit except for `need 
separation of concerns between SlowLog.toString and formatting of output in the 
shell`. The reason being shell is not able to use Gson as efficiently as 
keeping `Gson.toJson()` in toString of SlowLogRecord because we have some 
serialization filters (neither does shell have any good pretty print library 
that works with our POJO - AFAIK). shell or any client would receive 
`List<SlowLogRecord>` as output of `Admin.getSlowLogResponses()`. In a way, 
keeping `Gson.toJson()` within `toString()` would ensure all clients can print 
output in pretty print format. Do you think this is good to maintain uniform 
print format across all clients?
   
   No, I do not thing it's good to maintain uniform print format across all 
client. Anything that is providing display functionality for an object will 
need control over how the content is formatted.
   
   I maintain that pretty-printed json does not belong in the object's 
`toString` method. The abundantly common use-case for `toString` is the logging 
infrastructure, which has different requirements from the shell's user 
interface. Please just use the Apache Commons Lang `ToStringBuilder` (I prefer 
it's `SHORT_PREFIX_STYLE`) for that purpose.
   
   If you would like the object to control it's own formatting as a convenience 
to a variety of clients (something I find dubious but why not), I have no 
problem with putting this concern behind a separate method in `SlowLogRecord`, 
such as `toFormattedJson`. My main objection to this approach is that this 
introduces a dependency into `SlowLogRecord`, what is otherwise a simple POJO, 
onto an unrelated provider of json representation.
   
   Also, you may not have noticed that the shell has it's own class for 
handling display, called `Formatter`, under 
`hbase-shell/src/main/ruby/shell/formatter.rb`. I haven't used it myself, but 
it seems purpose-built to handle the use-case you have.
   

----------------------------------------------------------------
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:
[email protected]


With regards,
Apache Git Services

Reply via email to