[
https://issues.apache.org/jira/browse/YARN-4997?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15413907#comment-15413907
]
Daniel Templeton commented on YARN-4997:
----------------------------------------
Thanks for the patch, [~Tao Jie]. Overall looks good to me. A couple of minor
comments:
{code}
LOG.info(authorizer.getClass().getName() + " is destoryed.");
{code}
Probably best to make the log message at DEBUG level and only do the string
concat if debug is on.
{code}
if (queueInfo == null) {
authorizer.setPermission(allocsLoader.getDefaultPermissions(),
UserGroupInformation.getCurrentUser());
return;
}
{code}
Since that's in a small method, can we please use an _else_ instead of
returning from the _if_?
{code}
AccessControlList operationAcl = acls.get(
SchedulerUtils.toAccessType(operation));
{code}
Tiny quibble... Can we split the line on the equals instead of on the paren?
{code}
authorizer
.setPermission(permissions, UserGroupInformation.getCurrentUser());
{code}
Similarly, can we break on the comma here instead of the dot?
Finally, for the public and protected methods you added, please add javadocs.
> Update fair scheduler to use pluggable auth provider
> ----------------------------------------------------
>
> Key: YARN-4997
> URL: https://issues.apache.org/jira/browse/YARN-4997
> Project: Hadoop YARN
> Issue Type: Improvement
> Components: fairscheduler
> Affects Versions: 2.8.0
> Reporter: Daniel Templeton
> Assignee: Tao Jie
> Attachments: YARN-4997-001.patch, YARN-4997-002.patch
>
>
> Now that YARN-3100 has made the authorization pluggable, it should be
> supported by the fair scheduler. YARN-3100 only updated the capacity
> scheduler.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
