Andrew Wong has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8208 )

Change subject: itest: allow use of verifiers with EMCs that specify 
non-default block managers
......................................................................


Patch Set 5:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/8208/4/src/kudu/integration-tests/disk_failure-itest.cc
File src/kudu/integration-tests/disk_failure-itest.cc:

http://gerrit.cloudera.org:8080/#/c/8208/4/src/kudu/integration-tests/disk_failure-itest.cc@115
PS4, Line 115: }
             :
             : INSTANTIATE_TEST_CASE_P(DiskFailure, DiskFailureITest,
             :     ::testing::ValuesIn(BlockManager::block_manager_types()));
             :
> Can you encapsulate the list of available block managers in a helper method
Done


http://gerrit.cloudera.org:8080/#/c/8208/4/src/kudu/integration-tests/log_verifier.cc
File src/kudu/integration-tests/log_verifier.cc:

http://gerrit.cloudera.org:8080/#/c/8208/4/src/kudu/integration-tests/log_verifier.cc@59
PS4, Line 59: using cluster::ExternalMiniCluster;
> Don't need this (or gflags_declare.h) anymore?
Done


http://gerrit.cloudera.org:8080/#/c/8208/4/src/kudu/mini-cluster/external_mini_cluster.cc
File src/kudu/mini-cluster/external_mini_cluster.cc:

http://gerrit.cloudera.org:8080/#/c/8208/4/src/kudu/mini-cluster/external_mini_cluster.cc@134
PS4, Line 134:   daemon_bin_path_ = opts_.daemon_bin_path;
> Maybe we can move all of this to the ExternalMiniClusterOptions constructor
Hrm, I like the constructor idea since it aligns with the static Create/Open we 
use elsewhere. In that way, a static Create could modify/copy the options, and 
the EMC itself could keep a std::move()d copy. Regardless, I think that's work 
for another patch.



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iaa44eb33d0c025830f97a2ed7583c8186f915e94
Gerrit-Change-Number: 8208
Gerrit-PatchSet: 5
Gerrit-Owner: Andrew Wong <[email protected]>
Gerrit-Reviewer: Adar Dembo <[email protected]>
Gerrit-Reviewer: Andrew Wong <[email protected]>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <[email protected]>
Gerrit-Reviewer: Tidy Bot
Gerrit-Comment-Date: Fri, 06 Oct 2017 16:14:52 +0000
Gerrit-HasComments: Yes

Reply via email to