otterc commented on code in PR #2231:
URL:
https://github.com/apache/incubator-celeborn/pull/2231#discussion_r1456321778
##########
common/src/main/java/org/apache/celeborn/common/network/sasl/SaslClientBootstrap.java:
##########
@@ -84,8 +84,15 @@ public void doBootstrap(TransportClient client, Channel
channel) {
while (!saslClient.isComplete()) {
PbSaslRequest.Builder builder = PbSaslRequest.newBuilder();
if (firstToken) {
-
builder.setMethod(DIGEST_MD5).setAuthType(PbAuthType.CONNECTION_AUTH);
+ builder.setMethod(DIGEST_MD5);
}
+ // Setting the auth type to CONNECTION_AUTH for every message not just
the first one. This
+ // is
+ // because in Protobuf, for enums, the default value is the first
defined enum value which
+ // is the
+ // CLIENT_AUTH. It will be incorrect to set the auth type to
client_auth for every SASL
+ // message.
Review Comment:
K. I will add this undefined value but I think we should still set the the
auth type for every SASL message. It makes the implementation on the server
simpler and it's not increasing the payload.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
