[
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)
