[ https://issues.apache.org/jira/browse/YARN-2629?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14162665#comment-14162665 ]
Li Lu commented on YARN-2629: ----------------------------- Hi [~zjshen], I reviewed this patch (YARN-2629.3.patch), and it generally LGTM. However, I do have a few quick comments: 1. I noticed that you've changed a few method signatures, and removed the "throws IOException, YarnException" clause from the signatures. Meanwhile, in the method I noticed that you're catching all types of exceptions in one catch statement. I agree that this change saves some repeated lines of try-catch statements in various call sites, but is it OK catch all exceptions internally? (I think so, but would like to verify that. ) 2. Maybe we want to add one more test with no arguments on domain/view_acl/modify_acl? Timeline domain is a newly added feature, and maybe we'd like to make sure it would not break any existing distributed shell usages (esp. in some scripts)? 3. In Client.java: {code} private boolean toCreate = false; {code} This variable name looks unclear. Maybe we want to rename it so that people can easily connect it with timeline domains? > Make distributed shell use the domain-based timeline ACLs > --------------------------------------------------------- > > Key: YARN-2629 > URL: https://issues.apache.org/jira/browse/YARN-2629 > Project: Hadoop YARN > Issue Type: Sub-task > Components: timelineserver > Reporter: Zhijie Shen > Assignee: Zhijie Shen > Attachments: YARN-2629.1.patch, YARN-2629.2.patch, YARN-2629.3.patch > > > For demonstration the usage of this feature (YARN-2102), it's good to make > the distributed shell create the domain, and post its timeline entities into > this private space. -- This message was sent by Atlassian JIRA (v6.3.4#6332)