Sunil G commented on YARN-3250:

Hi [~rohithsharma] 
Thanks for the updated patch. Some minor nits.

1. In {{refreshClusterMaxPriority}}, 
checkRMStatus(user.getShortUserName(), argName,
        "refresh Cluster Max Priority");
we could pass *msg* as parameter instead of same string again.

2. Also do we need to write any error for failure cases of 
{{refreshClusterMaxPriority}} with {{RMAuditLogger}}?
3. I feel *getClusterMaxPriority* can be like *getMaxPriorityFromConf* as we 
already have a public api called *getMaxClusterLevelAppPriority*
4. Could you please add {{rm.stop()}} at the end of 
 new UsageInfo("",
+                  "Refresh cluster max priority of an application"))
I feel we can say "Refresh cluster max priority".

> Support admin cli interface in for Application Priority
> -------------------------------------------------------
>                 Key: YARN-3250
>                 URL: https://issues.apache.org/jira/browse/YARN-3250
>             Project: Hadoop YARN
>          Issue Type: Sub-task
>          Components: resourcemanager
>            Reporter: Sunil G
>            Assignee: Rohith Sharma K S
>         Attachments: 0001-YARN-3250-V1.patch, 0002-YARN-3250.patch
> Current Application Priority Manager supports only configuration via file. 
> To support runtime configurations for admin cli and REST, a common management 
> interface has to be added which can be shared with NodeLabelsManager. 

This message was sent by Atlassian JIRA

Reply via email to