Dan Burkert has posted comments on this change. Change subject: [rpc] WIP: Introduce configurable options to Messenger ......................................................................
Patch Set 1: (5 comments) http://gerrit.cloudera.org:8080/#/c/6520/1/src/kudu/rpc/messenger.cc File src/kudu/rpc/messenger.cc: Line 77: : connection_keepalive_time( It seems a little troublesome to have the defaults duplicated between the flags and here, but I don't really have a better idea. http://gerrit.cloudera.org:8080/#/c/6520/1/src/kudu/rpc/messenger.h File src/kudu/rpc/messenger.h: Line 81: struct MessengerBuilderOptions { Not quite seeing how this is distinguished from MessengerBuilder. They both seem to be a builder pattern, just different styles? Consider combining them by turning MessengerBuilder into a struct and add all the options from this into it. Line 93: std::string rpc_encryption; Could these flags instead by the parsed types, i.e. RpcAuthentication and RpcEncryption. http://gerrit.cloudera.org:8080/#/c/6520/1/src/kudu/rpc/negotiation.h File src/kudu/rpc/negotiation.h: Line 28: namespace security { unused? http://gerrit.cloudera.org:8080/#/c/6520/1/src/kudu/server/server_base.cc File src/kudu/server/server_base.cc: Line 122: DEFINE_string(rpc_certificate_file, "", whitespace -- To view, visit http://gerrit.cloudera.org:8080/6520 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I3685f137770d46f7c6537a37f76a0a6f71a00b11 Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Sailesh Mukil <sail...@cloudera.com> Gerrit-Reviewer: Dan Burkert <danburk...@apache.org> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Sailesh Mukil <sail...@cloudera.com> Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon <t...@apache.org> Gerrit-HasComments: Yes
