9uapaw commented on a change in pull request #3938:
URL: https://github.com/apache/hadoop/pull/3938#discussion_r808916419



##########
File path: 
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/RMAppManager.java
##########
@@ -467,28 +474,33 @@ private RMAppImpl createAndPopulateNewRMApp(
       if (scheduler instanceof CapacityScheduler) {
         String queueName = placementContext == null ?
             submissionContext.getQueue() : placementContext.getFullQueuePath();
-
-        String appName = submissionContext.getApplicationName();
-        CSQueue csqueue = ((CapacityScheduler) scheduler).getQueue(queueName);
-
-        if (csqueue == null && placementContext != null) {
-          //could be an auto created queue through queue mapping. Validate
-          // parent queue exists and has valid acls
-          String parentQueueName = placementContext.getParentQueue();
-          csqueue = ((CapacityScheduler) scheduler).getQueue(parentQueueName);
+        CapacityScheduler cs = (CapacityScheduler) scheduler;
+        CSQueue csqueue = cs.getQueue(queueName);
+        PrivilegedEntity privilegedEntity = getPrivilegedEntity(
+            csqueue == null ? queueName : csqueue.getQueuePath());
+
+        YarnAuthorizationProvider dynamicAuthorizer = null;
+        boolean ambiguous = false;
+        if (csqueue == null) {
+          try {
+            List<Permission> permissions =
+                
cs.getCapacitySchedulerQueueManager().getPermissionsForDynamicQueue(
+                    new QueuePath(queueName), cs.getConfiguration());
+            if (!permissions.isEmpty()) {
+              dynamicAuthorizer = new ConfiguredYarnAuthorizer();
+              dynamicAuthorizer.setPermission(permissions, userUgi);
+            }
+          } catch (SchedulerDynamicEditException e) {
+            ambiguous = true;

Review comment:
       I think a cleaner solution would be to catch the exception in 
getPermissionForDynamicQueue, and return an empty permission list in case of an 
error. I would add at least a debug log for the error just in case. It is 
rarely a good idea to ignore an exception. I do think it would be useful to see 
why ACLs are ignored for a dynamic queue.




-- 
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]



---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to