Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/21673 )
Change subject: [ranger] enhance the robustness of key retrieval ...................................................................... Patch Set 5: (7 comments) Almost there: just a few questions and nits. http://gerrit.cloudera.org:8080/#/c/21673/5//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/21673/5//COMMIT_MSG@9 PS5, Line 9: In scenarios where encryption keys are generated using Ranger Do you mean existing Kudu test scenarios or that was something else? http://gerrit.cloudera.org:8080/#/c/21673/5//COMMIT_MSG@15 PS5, Line 15: the new user nit: a new user http://gerrit.cloudera.org:8080/#/c/21673/5//COMMIT_MSG@21 PS5, Line 21: there are no logical changes Well, there is a change, actually -- now there is a retry :) If you've seen some test scenarios failing because of this from time to time, then I'd think that no extra test is needed (BTW, please add the name of the failing test into the commit message in this case). Otherwise, please at least verify manually that you don't see the issue anymore if you run the reproduction scenario a reasonable number of times in the same environment. If you already verified that, please add a note about that in the commit message. Thank you! http://gerrit.cloudera.org:8080/#/c/21673/5/src/kudu/ranger-kms/ranger_kms_client.cc File src/kudu/ranger-kms/ranger_kms_client.cc: http://gerrit.cloudera.org:8080/#/c/21673/5/src/kudu/ranger-kms/ranger_kms_client.cc@37 PS5, Line 37: key generate the key generation http://gerrit.cloudera.org:8080/#/c/21673/5/src/kudu/ranger-kms/ranger_kms_client.cc@114 PS5, Line 114: MonoTime now; As of PS5, this variable doesn't seem to be used anymore, just assigned at line 125. Consider removing it. http://gerrit.cloudera.org:8080/#/c/21673/5/src/kudu/ranger-kms/ranger_kms_client.cc@139 PS5, Line 139: Failed to generate server key nit: consider defining this constant at the upper level and using it in the LOG(WARNING) message at line 124. http://gerrit.cloudera.org:8080/#/c/21673/5/src/kudu/ranger-kms/ranger_kms_client.cc@139 PS5, Line 139: constexpr const char* nit: constexpr const char* const ? -- To view, visit http://gerrit.cloudera.org:8080/21673 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I1fd3263ad6ba6d8e444036bb7d2158986098cb4b Gerrit-Change-Number: 21673 Gerrit-PatchSet: 5 Gerrit-Owner: KeDeng <[email protected]> Gerrit-Reviewer: Alexey Serbin <[email protected]> Gerrit-Reviewer: KeDeng <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Yingchun Lai <[email protected]> Gerrit-Reviewer: Zoltan Chovan <[email protected]> Gerrit-Comment-Date: Sat, 24 Aug 2024 05:30:39 +0000 Gerrit-HasComments: Yes
