Zoltan Chovan has posted comments on this change. ( http://gerrit.cloudera.org:8080/18645 )
Change subject: [ranger-kms] part2: mini_ranger init ...................................................................... Patch Set 7: (14 comments) http://gerrit.cloudera.org:8080/#/c/18645/7/src/kudu/mini-cluster/external_mini_cluster.h File src/kudu/mini-cluster/external_mini_cluster.h: http://gerrit.cloudera.org:8080/#/c/18645/7/src/kudu/mini-cluster/external_mini_cluster.h@44 PS7, Line 44: //#include "kudu/ranger-kms/mini_ranger_kms.h" > nit: remove Ack http://gerrit.cloudera.org:8080/#/c/18645/7/src/kudu/mini-cluster/external_mini_cluster.h@228 PS7, Line 228: // Default: false > nit: add a period at the end Ack http://gerrit.cloudera.org:8080/#/c/18645/7/src/kudu/postgres/mini_postgres.h File src/kudu/postgres/mini_postgres.h: http://gerrit.cloudera.org:8080/#/c/18645/7/src/kudu/postgres/mini_postgres.h@84 PS7, Line 84: bool firstRun() const { return !Env::Default()->FileExists(pg_root()); } > nit: IsFirstRun() Done http://gerrit.cloudera.org:8080/#/c/18645/7/src/kudu/postgres/mini_postgres.h@86 PS7, Line 86: bool isRunning() const { return process_ && process_->IsStarted(); } > nit: IsRunning() Done http://gerrit.cloudera.org:8080/#/c/18645/7/src/kudu/postgres/mini_postgres.cc File src/kudu/postgres/mini_postgres.cc: http://gerrit.cloudera.org:8080/#/c/18645/7/src/kudu/postgres/mini_postgres.cc@101 PS7, Line 101: Status MiniPostgres::Stop() { > nit: indent Done http://gerrit.cloudera.org:8080/#/c/18645/7/src/kudu/ranger-kms/CMakeLists.txt File src/kudu/ranger-kms/CMakeLists.txt: http://gerrit.cloudera.org:8080/#/c/18645/7/src/kudu/ranger-kms/CMakeLists.txt@18 PS7, Line 18: # todo @zchovan maybe we need to add a client for mini kms? > nit: todo(zchovan) nope, thx http://gerrit.cloudera.org:8080/#/c/18645/7/src/kudu/ranger-kms/mini_ranger_kms.h File src/kudu/ranger-kms/mini_ranger_kms.h: http://gerrit.cloudera.org:8080/#/c/18645/7/src/kudu/ranger-kms/mini_ranger_kms.h@45 PS7, Line 45: namespace ranger_kms { > nit: it should be rangerkms instead Ack http://gerrit.cloudera.org:8080/#/c/18645/7/src/kudu/ranger-kms/mini_ranger_kms.h@48 PS7, Line 48: public: > nit: indent should be 1 for public and private, and 2 for the functions Done http://gerrit.cloudera.org:8080/#/c/18645/7/src/kudu/ranger-kms/mini_ranger_kms.h@103 PS7, Line 103: // creates keystores for the KMS > nit: start with a capital letter and add a period at the end (also for the Ack http://gerrit.cloudera.org:8080/#/c/18645/7/src/kudu/ranger-kms/mini_ranger_kms.cc File src/kudu/ranger-kms/mini_ranger_kms.cc: http://gerrit.cloudera.org:8080/#/c/18645/7/src/kudu/ranger-kms/mini_ranger_kms.cc@333 PS7, Line 333: // curl.set_auth(CurlAuthType::BASIC, "user1", "t1e2s3t4"); > nit: remove Done http://gerrit.cloudera.org:8080/#/c/18645/7/src/kudu/ranger-kms/mini_ranger_kms_configs.h File src/kudu/ranger-kms/mini_ranger_kms_configs.h: http://gerrit.cloudera.org:8080/#/c/18645/7/src/kudu/ranger-kms/mini_ranger_kms_configs.h@392 PS7, Line 392: <property> > nit: use spaces instead of tabs (here and below) Done http://gerrit.cloudera.org:8080/#/c/18645/7/src/kudu/ranger/mini_ranger.h File src/kudu/ranger/mini_ranger.h: http://gerrit.cloudera.org:8080/#/c/18645/7/src/kudu/ranger/mini_ranger.h@122 PS7, Line 122: bool isRunning() const { return process_ && process_->IsStarted(); } > nit: IsRunning() Done http://gerrit.cloudera.org:8080/#/c/18645/7/src/kudu/ranger/mini_ranger.h@168 PS7, Line 168: // Directory in which to put all our stuff. > nit: indent Done http://gerrit.cloudera.org:8080/#/c/18645/7/src/kudu/util/env_util.h File src/kudu/util/env_util.h: http://gerrit.cloudera.org:8080/#/c/18645/7/src/kudu/util/env_util.h@85 PS7, Line 85: Status CopyDirectory(Env* env, const std::string& source_path, const std::string& dest_path, > nit: add doc comment Done -- To view, visit http://gerrit.cloudera.org:8080/18645 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I11617468245068dd732fb3f2578bb086b2f6024f Gerrit-Change-Number: 18645 Gerrit-PatchSet: 7 Gerrit-Owner: Zoltan Chovan <[email protected]> Gerrit-Reviewer: Attila Bukor <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Reviewer: Zoltan Chovan <[email protected]> Gerrit-Comment-Date: Thu, 21 Jul 2022 17:26:12 +0000 Gerrit-HasComments: Yes
