Junping Du commented on YARN-1250:

Thanks [~zjshen] for updating the patch! Some comments:

bq.  The reason is that when user doesn't have the access to the app, we show 
partial info (excluding the attempt details), and when user doesn't have the 
access to the attempt/container, we throw exception.
Sounds good.

bq. We have corresponding INFO level log message to show whether system metrics 
publisher is enabled or not. If putting entity is failed, we will log an error 
log. And if DEBUG is enabled, we're going to record done all the entities that 
are going to be put. Hence I think it should be enough for testing and 
In a system prospective, it never have too many warnings when hitting into 
abnormal status. We should immediately log the wrong situation no matter if a 
similar exception get logged before. In an API design prospective, the caller 
should have some expectation for some bottom-line behavior - like trigger some 
events. If this call is totally neglected because of configuration (no matter 
right or wrong configuration), the caller should be aware (still, better to log 
properly). If we do think the call can be reasonably neglected, we should at 
least document the reason so others will pay attention to it when reusing this 
method in future.

+        if (callerUGI != null && callerUGI.getShortUserName().equals("user3")) 
+          // The exception is expected
+          Assert.fail();
+        }
We can just move "Assert.fail();" out of if block because if exception isn't 
thrown as expected, we should just get failed directly without affected by if 
case. In addition, I saw many duplicated code in tests here. Mind to refactor 
them to a common method?

> Generic history service should support application-acls
> -------------------------------------------------------
>                 Key: YARN-1250
>                 URL: https://issues.apache.org/jira/browse/YARN-1250
>             Project: Hadoop YARN
>          Issue Type: Sub-task
>            Reporter: Vinod Kumar Vavilapalli
>            Assignee: Zhijie Shen
>         Attachments: GenericHistoryACLs.pdf, YARN-1250.1.patch, 
> YARN-1250.2.patch, YARN-1250.3.patch, YARN-1250.4.patch, YARN-1250.5.patch

This message was sent by Atlassian JIRA

Reply via email to