[ 
https://issues.apache.org/jira/browse/YARN-4997?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15415585#comment-15415585
 ] 

Daniel Templeton commented on YARN-4997:
----------------------------------------

Thanks for the update.  I still have a few nits to pick. :)

First, you're still missing some javadocs, and some of the javadocs you added 
is missing the {{@return}} tags.

In this code:

{code}
  protected List<Permission> getDefaultPermissions() {
    if (defaultPermissions == null) {
      List<Permission> permissions = new ArrayList<Permission>();
      Map<AccessType, AccessControlList> acls =
          new HashMap<AccessType, AccessControlList>();
      for (QueueACL acl : QueueACL.values()) {
        acls.put(SchedulerUtils.toAccessType(acl), EVERYBODY_ACL);
      }
      permissions.add(new Permission(
          new PrivilegedEntity(EntityType.QUEUE, ROOT), acls));
      defaultPermissions = permissions;
    }
    return defaultPermissions;
  }
{code}

Why the intermediate {{permissions}} variable?  It smells like maybe an attempt 
at thread safety.  If so, it's not enough.  If not, can we drop the extra 
variable since it looks suspicious?

Here:

{code}
-    public void onReload(AllocationConfiguration info);
+    void onReload(AllocationConfiguration info) throws IOException;
{code}

I'd rather we keep the {{public}} keyword.  It's optional for interfaces, but 
it's easier to read with the keyword there.

> 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, 
> YARN-4997-003.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: yarn-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: yarn-issues-h...@hadoop.apache.org

Reply via email to