You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "Dan Burkert (Code Review)" <ge...@cloudera.org> on 2017/01/21 04:47:58 UTC
[kudu-CR] TLS-negotiation [3/n]: rename negotiation protobuf messages
Hello Adar Dembo, Todd Lipcon, Alexey Serbin,
I'd like you to do a code review. Please visit
http://gerrit.cloudera.org:8080/5757
to review the following change.
Change subject: TLS-negotiation [3/n]: rename negotiation protobuf messages
......................................................................
TLS-negotiation [3/n]: rename negotiation protobuf messages
SASL negotiation in the RPC layer has evolved to encompass more than
just SASL. Today it includes RpcFeatureFlag negotiation, and in the
future it may include TLS and authn token negotiation. Consequently,
this commit renames the message to reflect its role.
Change-Id: Ie24eec6e632c6e5064afa631eccb3877425fe354
---
M java/kudu-client/src/main/java/org/apache/kudu/client/SecureRpcHelper.java
M src/kudu/rpc/blocking_ops.h
M src/kudu/rpc/rpc_header.proto
M src/kudu/rpc/sasl_client.cc
M src/kudu/rpc/sasl_client.h
M src/kudu/rpc/sasl_helper.cc
M src/kudu/rpc/sasl_helper.h
M src/kudu/rpc/sasl_server.cc
M src/kudu/rpc/sasl_server.h
9 files changed, 85 insertions(+), 87 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/57/5757/1
--
To view, visit http://gerrit.cloudera.org:8080/5757
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ie24eec6e632c6e5064afa631eccb3877425fe354
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
[kudu-CR] TLS-negotiation [3/n]: rename negotiation protobuf messages
Posted by "Todd Lipcon (Code Review)" <ge...@cloudera.org>.
Todd Lipcon has posted comments on this change.
Change subject: TLS-negotiation [3/n]: rename negotiation protobuf messages
......................................................................
Patch Set 3: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/5757
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie24eec6e632c6e5064afa631eccb3877425fe354
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: No
[kudu-CR] TLS-negotiation [3/n]: rename negotiation protobuf messages
Posted by "Todd Lipcon (Code Review)" <ge...@cloudera.org>.
Todd Lipcon has posted comments on this change.
Change subject: TLS-negotiation [3/n]: rename negotiation protobuf messages
......................................................................
Patch Set 2:
(4 comments)
http://gerrit.cloudera.org:8080/#/c/5757/2/src/kudu/rpc/sasl_server.cc
File src/kudu/rpc/sasl_server.cc:
Line 176: TRACE("Waiting for next SASL message...");
next negotiation message?
PS2, Line 200: SASL request.
same
PS2, Line 253: SaslMessage
maybe this should be renamed?
http://gerrit.cloudera.org:8080/#/c/5757/2/src/kudu/rpc/sasl_server.h
File src/kudu/rpc/sasl_server.h:
Line 122: // Encode and send the specified SASL message to the client.
nit: this should say 'negotiation message' or something
--
To view, visit http://gerrit.cloudera.org:8080/5757
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie24eec6e632c6e5064afa631eccb3877425fe354
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: Yes
[kudu-CR] TLS-negotiation [3/n]: rename negotiation protobuf messages
Posted by "Dan Burkert (Code Review)" <ge...@cloudera.org>.
Dan Burkert has posted comments on this change.
Change subject: TLS-negotiation [3/n]: rename negotiation protobuf messages
......................................................................
Patch Set 3:
(4 comments)
http://gerrit.cloudera.org:8080/#/c/5757/2/src/kudu/rpc/sasl_server.cc
File src/kudu/rpc/sasl_server.cc:
Line 176: TRACE("Waiting for next Negotiation message...");
> next negotiation message?
Done
PS2, Line 200: gotiation req
> same
Done
PS2, Line 253:
> maybe this should be renamed?
Done
http://gerrit.cloudera.org:8080/#/c/5757/2/src/kudu/rpc/sasl_server.h
File src/kudu/rpc/sasl_server.h:
Line 122:
> nit: this should say 'negotiation message' or something
Done
--
To view, visit http://gerrit.cloudera.org:8080/5757
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie24eec6e632c6e5064afa631eccb3877425fe354
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: Yes
[kudu-CR] TLS-negotiation [3/n]: rename negotiation protobuf messages
Posted by "Dan Burkert (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/5757
to look at the new patch set (#2).
Change subject: TLS-negotiation [3/n]: rename negotiation protobuf messages
......................................................................
TLS-negotiation [3/n]: rename negotiation protobuf messages
SASL negotiation in the RPC layer has evolved to encompass more than
just SASL. Today it includes RpcFeatureFlag negotiation, and in the
future it may include TLS and authn token negotiation. Consequently,
this commit renames the message to reflect its role.
Change-Id: Ie24eec6e632c6e5064afa631eccb3877425fe354
---
M java/kudu-client/src/main/java/org/apache/kudu/client/SecureRpcHelper.java
M src/kudu/rpc/blocking_ops.cc
M src/kudu/rpc/blocking_ops.h
M src/kudu/rpc/rpc_header.proto
M src/kudu/rpc/sasl_client.cc
M src/kudu/rpc/sasl_client.h
M src/kudu/rpc/sasl_helper.cc
M src/kudu/rpc/sasl_helper.h
M src/kudu/rpc/sasl_server.cc
M src/kudu/rpc/sasl_server.h
10 files changed, 87 insertions(+), 89 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/57/5757/2
--
To view, visit http://gerrit.cloudera.org:8080/5757
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ie24eec6e632c6e5064afa631eccb3877425fe354
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
[kudu-CR] TLS-negotiation [3/n]: rename negotiation protobuf messages
Posted by "Dan Burkert (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/5757
to look at the new patch set (#3).
Change subject: TLS-negotiation [3/n]: rename negotiation protobuf messages
......................................................................
TLS-negotiation [3/n]: rename negotiation protobuf messages
SASL negotiation in the RPC layer has evolved to encompass more than
just SASL. Today it includes RpcFeatureFlag negotiation, and in the
future it may include TLS and authn token negotiation. Consequently,
this commit renames the message to reflect its role.
Change-Id: Ie24eec6e632c6e5064afa631eccb3877425fe354
---
M java/kudu-client/src/main/java/org/apache/kudu/client/SecureRpcHelper.java
M src/kudu/rpc/blocking_ops.cc
M src/kudu/rpc/blocking_ops.h
M src/kudu/rpc/constants.h
M src/kudu/rpc/rpc_header.proto
M src/kudu/rpc/sasl_client.cc
M src/kudu/rpc/sasl_client.h
M src/kudu/rpc/sasl_helper.cc
M src/kudu/rpc/sasl_helper.h
M src/kudu/rpc/sasl_server.cc
M src/kudu/rpc/sasl_server.h
11 files changed, 143 insertions(+), 135 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/57/5757/3
--
To view, visit http://gerrit.cloudera.org:8080/5757
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ie24eec6e632c6e5064afa631eccb3877425fe354
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
[kudu-CR] TLS-negotiation [3/n]: rename negotiation protobuf messages
Posted by "Todd Lipcon (Code Review)" <ge...@cloudera.org>.
Todd Lipcon has submitted this change and it was merged.
Change subject: TLS-negotiation [3/n]: rename negotiation protobuf messages
......................................................................
TLS-negotiation [3/n]: rename negotiation protobuf messages
SASL negotiation in the RPC layer has evolved to encompass more than
just SASL. Today it includes RpcFeatureFlag negotiation, and in the
future it may include TLS and authn token negotiation. Consequently,
this commit renames the message to reflect its role.
Change-Id: Ie24eec6e632c6e5064afa631eccb3877425fe354
Reviewed-on: http://gerrit.cloudera.org:8080/5757
Tested-by: Kudu Jenkins
Reviewed-by: Todd Lipcon <to...@apache.org>
---
M java/kudu-client/src/main/java/org/apache/kudu/client/SecureRpcHelper.java
M src/kudu/rpc/blocking_ops.cc
M src/kudu/rpc/blocking_ops.h
M src/kudu/rpc/constants.h
M src/kudu/rpc/rpc_header.proto
M src/kudu/rpc/sasl_client.cc
M src/kudu/rpc/sasl_client.h
M src/kudu/rpc/sasl_helper.cc
M src/kudu/rpc/sasl_helper.h
M src/kudu/rpc/sasl_server.cc
M src/kudu/rpc/sasl_server.h
11 files changed, 143 insertions(+), 135 deletions(-)
Approvals:
Todd Lipcon: Looks good to me, approved
Kudu Jenkins: Verified
--
To view, visit http://gerrit.cloudera.org:8080/5757
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: Ie24eec6e632c6e5064afa631eccb3877425fe354
Gerrit-PatchSet: 4
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
[kudu-CR] TLS-negotiation [3/n]: rename negotiation protobuf messages
Posted by "Dan Burkert (Code Review)" <ge...@cloudera.org>.
Dan Burkert has posted comments on this change.
Change subject: TLS-negotiation [3/n]: rename negotiation protobuf messages
......................................................................
Patch Set 2:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/5757/1/src/kudu/rpc/blocking_ops.h
File src/kudu/rpc/blocking_ops.h:
Line 42: Status EnsureBlockingMode(const Socket* sock);
> warning: parameter 'sock' is const-qualified in the function declaration; c
Done
--
To view, visit http://gerrit.cloudera.org:8080/5757
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie24eec6e632c6e5064afa631eccb3877425fe354
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: Yes