Will Berkeley has posted comments on this change. ( http://gerrit.cloudera.org:8080/12450 )
Change subject: [tools] Add to 'kudu remote_replica list' the ability to exclude schema and to filter on tablet id, table name ...................................................................... Patch Set 1: (6 comments) http://gerrit.cloudera.org:8080/#/c/12450/1//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/12450/1//COMMIT_MSG@7 PS1, Line 7: [tools] Add to 'kudu remote_replica list' the ability to exclude schema and to filter on tablet id, table name > Nit: wrap/rewrite? Fine. http://gerrit.cloudera.org:8080/#/c/12450/1/src/kudu/tools/kudu-tool-test.cc File src/kudu/tools/kudu-tool-test.cc: http://gerrit.cloudera.org:8080/#/c/12450/1/src/kudu/tools/kudu-tool-test.cc@1926 PS1, Line 1926: no_need_schema_info > include_schema=false Done http://gerrit.cloudera.org:8080/#/c/12450/1/src/kudu/tools/kudu-tool-test.cc@1935 PS1, Line 1935: // Test we lose the tablet when matching on the wrong tablet id or the wrong : // table name. : string stdout; : NO_FATALS(RunActionStdoutString( : Substitute("remote_replica list $0 --table_name=foo", ts_addr), : &stdout)); : ASSERT_STR_NOT_CONTAINS(stdout, : Substitute("Tablet id: $0", : tablet_status.tablet_id())); : stdout.clear(); : NO_FATALS(RunActionStdoutString( : Substitute("remote_replica list $0 --tablets=foo", ts_addr), : &stdout)); : ASSERT_STR_NOT_CONTAINS(stdout, : Substitute("Tablet id: $0", : tablet_status.tablet_id())); > Think it's worth testing for the cross of the two? e.g. `--table_name=<actu Not really. I did test that manually, though. http://gerrit.cloudera.org:8080/#/c/12450/1/src/kudu/tools/tool_action_remote_replica.cc File src/kudu/tools/tool_action_remote_replica.cc: http://gerrit.cloudera.org:8080/#/c/12450/1/src/kudu/tools/tool_action_remote_replica.cc@188 PS1, Line 188: te > to Done http://gerrit.cloudera.org:8080/#/c/12450/1/src/kudu/tools/tool_action_remote_replica.cc@294 PS1, Line 294: std::unordered_set<string> tablet_ids = strings::Split(FLAGS_tablets, ","); > Nit: using Done http://gerrit.cloudera.org:8080/#/c/12450/1/src/kudu/tools/tool_action_remote_replica.cc@464 PS1, Line 464: string(""), : string("Comma-separated list of tablet IDs used to " : "filter the list of replicas")) > Why can't these go in the DECLARE? They are overriding the defaults from the DEFINE for this tool action, specifically. That can't be done in the DECLARE. -- To view, visit http://gerrit.cloudera.org:8080/12450 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I462515f1bc3e8487185aebb6cb99d1c5c00cea40 Gerrit-Change-Number: 12450 Gerrit-PatchSet: 1 Gerrit-Owner: Will Berkeley <wdberke...@gmail.com> Gerrit-Reviewer: Adar Dembo <a...@cloudera.com> Gerrit-Reviewer: Alexey Serbin <aser...@cloudera.com> Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Todd Lipcon <t...@apache.org> Gerrit-Reviewer: Will Berkeley <wdberke...@gmail.com> Gerrit-Comment-Date: Thu, 14 Feb 2019 20:35:00 +0000 Gerrit-HasComments: Yes
