Adar Dembo has posted comments on this change.

Change subject: [tools] updated insert-generated-rows tool
......................................................................


Patch Set 6:

(1 comment)

Would be nice to see a small integration test that runs the tool against a 
cluster then verifies that it indeed inserted rows. Our CLI tools have a 
tendency to bit-rot because of the lack of tests; we're trying to change that 
with the new CLI tool.

On that note, what do you think of incorporating this as an action in the new 
CLI tool? You'll get most of main() for free, but it means this will ship to 
users. Do you think it'd a useful tool for them?

http://gerrit.cloudera.org:8080/#/c/4412/6/src/kudu/tools/insert-generated-rows.cc
File src/kudu/tools/insert-generated-rows.cc:

Line 116: DEFINE_string(master_address, "localhost:7051",
Nit: can we change this to master_addresses?


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I332927c0b928c9c4fb81a8e26f5c9ed7565299ad
Gerrit-PatchSet: 6
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin <aser...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <a...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <aser...@cloudera.com>
Gerrit-Reviewer: David Ribeiro Alves <dral...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon <t...@apache.org>
Gerrit-HasComments: Yes

Reply via email to