Alexey Serbin has posted comments on this change. Change subject: [TLS cert management] security service implementation ......................................................................
Patch Set 1: (9 comments) http://gerrit.cloudera.org:8080/#/c/5674/1/src/kudu/security/security_service.cc File src/kudu/security/security_service.cc: PS1, Line 61: (s) > this macro evaluates 's' a bunch of times, which is no good if it has side Done PS1, Line 77: ((s).ToString() > this is redundant with the string which already ends up in the statuspb (se Done PS1, Line 86: ConvertFromPBFormat > usual style is 'DataFormatFromPB' Done Line 88: if (PREDICT_FALSE(!format)) { > just CHECK or DCHECK Done PS1, Line 111: server->result_tracker() > hm, given that this .cc depends on kudu/rpc, I think we probably need it to I separated this into securit_ca library. Line 136: unique_ptr<string> ca_cert_str(new string); > this is a bit strange - I think a more readable way with the same perf woul Looks good -- I think the performance of this could be a little bit better. Line 167: unique_ptr<string> cert_str(new string); > see above Done http://gerrit.cloudera.org:8080/#/c/5674/1/src/kudu/security/security_service.h File src/kudu/security/security_service.h: Line 1: // Licensed to the Apache Software Foundation (ASF) under one > think it makes sense to name this file specifically for the cert-signing se Done PS1, Line 42: master::Master > this cyclic reference is a no-no. Good catch. It does not require master::Master, but just kudu::server::ServerBase to provide metrics and result tracker for the RPC server stub. -- To view, visit http://gerrit.cloudera.org:8080/5674 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I4e4319c752ce64c53046db5d29c3d50306bdcdc5 Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Alexey Serbin <aser...@cloudera.com> Gerrit-Reviewer: Alexey Serbin <aser...@cloudera.com> Gerrit-Reviewer: Dan Burkert <danburk...@apache.org> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy <mpe...@apache.org> Gerrit-Reviewer: Todd Lipcon <t...@apache.org> Gerrit-HasComments: Yes
