Fengling Wang has posted comments on this change. ( http://gerrit.cloudera.org:8080/11727 )
Change subject: [location_awareness] Add ts location to TSInfoPB ...................................................................... Patch Set 2: (1 comment) I'm thinking maybe the test is better added to ts_location_assignment-itest.cc because it seems more relevant to this patch and it uses ExternalMiniCluster that enables passing location_mapping into its opts. Am I understanding correctly? Thanks. http://gerrit.cloudera.org:8080/#/c/11727/2/src/kudu/integration-tests/table_locations-itest.cc File src/kudu/integration-tests/table_locations-itest.cc: http://gerrit.cloudera.org:8080/#/c/11727/2/src/kudu/integration-tests/table_locations-itest.cc@140 PS2, Line 140: FLAGS_location_mapping_cmd = strings::Substitute("$0 $1", location_cmd_path, location); > Could you actually configure this using 'opts'? The problem is that if this Thanks for the suggestion. Makes sense to me. One thing I'm not sure whether to do this is: The InternalMiniCluster has no 'extra_master_flags' in its opts. If I add extra_master_flags, there will be also non-trivial work done in InternalMiniCluster::StartMasters, just like what is done in ExternalMiniCluster::StartMasters. So do I want to add such chunk into InternalMiniCluster or maybe switch to some other itest that uses ExternalMiniCluster? Thanks. -- To view, visit http://gerrit.cloudera.org:8080/11727 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ic5865cb47698817097a7f62c968c5cfd80e07259 Gerrit-Change-Number: 11727 Gerrit-PatchSet: 2 Gerrit-Owner: Fengling Wang <[email protected]> Gerrit-Reviewer: Adar Dembo <[email protected]> Gerrit-Reviewer: Alexey Serbin <[email protected]> Gerrit-Reviewer: Fengling Wang <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Will Berkeley <[email protected]> Gerrit-Comment-Date: Mon, 22 Oct 2018 22:58:46 +0000 Gerrit-HasComments: Yes
