Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/14273 )
Change subject: add a tool to create table ...................................................................... Patch Set 5: (2 comments) http://gerrit.cloudera.org:8080/#/c/14273/5/src/kudu/tools/tool_action_table.cc File src/kudu/tools/tool_action_table.cc: http://gerrit.cloudera.org:8080/#/c/14273/5/src/kudu/tools/tool_action_table.cc@897 PS5, Line 897: RETURN_NOT_OK(reader.ExtractString(reader.root(), "TableName", &table_name)); > I'm Sorry, I don't understand this place. The JSON schema is provided to th Yes, the idea is that rather than doing this piece-by-piece parsing with JsonReader, you define the user's create table schema as a protobuf object, then use google::protobuf::util::JsonStringToMessage to convert it into the PB object. Then you use the PB object in this function to extract the various bits you need for KuduTableCreator. http://gerrit.cloudera.org:8080/#/c/14273/5/src/kudu/tools/tool_action_table.cc@1392 PS5, Line 1392: .AddRequiredParameter({ kCreateFileNameArg, "Name of the table file" }) > At first I want to define a JSON object, but the object is too long, it's v I agree that it's easy to make a mistake when parsing the JSON, but I just don't think a CLI tool should default to reading from a file like this. And if you switch it to read the JSON as the last argument, it's really easy to use shell redirection to get that argument from a file. Put another way, reading from an argument makes the tool more composable: you can either read the JSON object from the command line or a file. -- To view, visit http://gerrit.cloudera.org:8080/14273 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Iacd9ae64dc07e3da6efe840e4bbcfaf3df0ecdc7 Gerrit-Change-Number: 14273 Gerrit-PatchSet: 5 Gerrit-Owner: YangSong <[email protected]> Gerrit-Reviewer: Adar Dembo <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Reviewer: YangSong <[email protected]> Gerrit-Comment-Date: Mon, 23 Sep 2019 03:55:41 +0000 Gerrit-HasComments: Yes
