Attila Bukor 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 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 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() 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() 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 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) is this still needed? 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 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 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 others where it's missing) 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 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) 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() 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 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 -- 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-Comment-Date: Thu, 21 Jul 2022 16:39:26 +0000 Gerrit-HasComments: Yes
