[ https://issues.apache.org/jira/browse/GIRAPH-211?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13459279#comment-13459279 ]
Avery Ching commented on GIRAPH-211: ------------------------------------ Eugene, this looks nice! I still see some checkstyle stuff but you're working through it I guess. Overall, the design is good. If we think about improvements we can make them later on. Questions/Comments: For consistency, can you please convert multi-line comments like /** Whether to use SASL with DIGEST and Hadoop Job Tokens to authenticate * and authorize Netty BSP Clients to Servers. */ to /** * Whether to use SASL with DIGEST and Hadoop Job Tokens to authenticate * and authorize Netty BSP Clients to Servers. */ Can we get rename Authorize to AuthorizeServerHandler or something else more descriptive? NettyClient.java - 372: Please wrap the LOG.info() with if (LOG.isInfoEnabled()). - 545-557: Can't we just go through the regular netty request part of the code? We don't need to have -2 here and can just submit the destWorkerId? SASL_COMPLETE -> SASL_COMPLETE_REQUEST? SaslTokenMessage.java can we call is SaslTokenMessageRequest? SaslComplete.java can we call it SaslCompleteRequest to match the other names? SaslComplete.java - 29-34: Why not get rid of these? SaslTokenMessage.java: - 86: Extra line Thanks again, this was a lot of work! > Add secure authentication to Netty IPC > -------------------------------------- > > Key: GIRAPH-211 > URL: https://issues.apache.org/jira/browse/GIRAPH-211 > Project: Giraph > Issue Type: Improvement > Reporter: Eugene Koontz > Assignee: Eugene Koontz > Fix For: 0.2.0 > > Attachments: GIRAPH-211.patch, GIRAPH-211.patch, GIRAPH-211.patch, > GIRAPH-211.patch, GIRAPH-211.patch, GIRAPH-211.patch, GIRAPH-211.patch, > GIRAPH-211-proposal.txt > > > Gianmarco De Francisci Morales asked on the user list: > bq. I am getting the exception in the subject when running my giraph program > bq. on a cluster with Kerberos authentication. > This leads to the idea of having Kerberos authentication supported within > GIRAPH. Hopefully it would use our fast GIRAPH-37 IPC, but could also > interoperate with Hadoop security. -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators For more information on JIRA, see: http://www.atlassian.com/software/jira