Wang Xixu has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/18330 )

Change subject: [tool] Fix outputs 'kudu pbc dump instance' when using 
-json/-json_pretty
......................................................................


Patch Set 14:

(5 comments)

http://gerrit.cloudera.org:8080/#/c/18330/14//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/18330/14//COMMIT_MSG@12
PS14, Line 12: These two commands' outputs about the 'uuid' field are not the 
same
It is better to describe the difference of the first command and the second 
command.


http://gerrit.cloudera.org:8080/#/c/18330/14//COMMIT_MSG@14
PS14, Line 14: --debug/default' mode shows uuid's plain text while 
'--debug/default'
Here you need to compare '--debug/default' mode with '-json/-json_pretty' mode.


http://gerrit.cloudera.org:8080/#/c/18330/14/src/kudu/util/pb_util.h
File src/kudu/util/pb_util.h:

http://gerrit.cloudera.org:8080/#/c/18330/14/src/kudu/util/pb_util.h@427
PS14, Line 427:   Status Dump(std::ostream* os, Format format, DumpFlag flag = 
DumpFlag::DECODE_UUID);
Use DECODE_UUID as the default value is a little confusing. There is a DumpFlag 
naming DEFAULT.
Maybe you can rename DEFAULT as ENCODED_UUID.


http://gerrit.cloudera.org:8080/#/c/18330/14/src/kudu/util/pb_util.cc
File src/kudu/util/pb_util.cc:

http://gerrit.cloudera.org:8080/#/c/18330/14/src/kudu/util/pb_util.cc@990
PS14, Line 990: use base64 decode it
decode it from base64 format


http://gerrit.cloudera.org:8080/#/c/18330/14/src/kudu/util/pb_util.cc@994
PS14, Line 994:   // Maybe it's necessary to consider other 'bytes' fields 
which can be dumped.
If the whole message is encoded in base64 format, can we decode whole message 
from base64 format not only decode one field?



--
To view, visit http://gerrit.cloudera.org:8080/18330
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia21afb03d9b7b4b2d4ea5aaa642701451282bebf
Gerrit-Change-Number: 18330
Gerrit-PatchSet: 14
Gerrit-Owner: Yuqi Du <[email protected]>
Gerrit-Reviewer: Abhishek Chennaka <[email protected]>
Gerrit-Reviewer: Alexey Serbin <[email protected]>
Gerrit-Reviewer: Ashwani Raina <[email protected]>
Gerrit-Reviewer: KeDeng <[email protected]>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Wang Xixu <[email protected]>
Gerrit-Reviewer: Yifan Zhang <[email protected]>
Gerrit-Reviewer: Yingchun Lai <[email protected]>
Gerrit-Reviewer: Yuqi Du <[email protected]>
Gerrit-Comment-Date: Mon, 05 Jun 2023 06:59:20 +0000
Gerrit-HasComments: Yes

Reply via email to