[ 
https://issues.apache.org/jira/browse/YARN-1461?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13868505#comment-13868505
 ] 

Zhijie Shen commented on YARN-1461:
-----------------------------------

Have a look at the new patch. Here're some comments.

1. Rename it to "ScopeProto"?
{code}
+  enum Scope {
{code}

2. The previous pattern of defining enum in proto is to have non proto 
corresponding enum, and map them one-to-one. It avoid using proto object in 
GetApplicationsRequest.
{code}
+  @Override
+  public Scope getScope() {
+    initScope();
+    return this.scope;
+  }
+
+  public void setScope(Scope scope) {
+    maybeInitBuilder();
+    if (scope == null) {
+      builder.clearScope();
+    }
+    this.scope = scope;
+  }
{code}

3. Not sure about Hadoop coding guideline allows non-ASCII char in the source, 
which may troubles people who use ASCII editor. How about using "(char) an 
arbitrary short" instead?
{code}
+    tags.add("©");
{code}

4. Need to document the new param
{code}
+      @QueryParam("tags") Set<String> tags) {
{code}

5. Is the test failure due to the change of access check? Would you please have 
a look at it?

> RM API and RM changes to handle tags for running jobs
> -----------------------------------------------------
>
>                 Key: YARN-1461
>                 URL: https://issues.apache.org/jira/browse/YARN-1461
>             Project: Hadoop YARN
>          Issue Type: Sub-task
>          Components: resourcemanager
>    Affects Versions: 2.2.0
>            Reporter: Karthik Kambatla
>            Assignee: Karthik Kambatla
>         Attachments: yarn-1461-1.patch, yarn-1461-2.patch, yarn-1461-3.patch, 
> yarn-1461-4.patch, yarn-1461-5.patch, yarn-1461-6.patch, yarn-1461-6.patch, 
> yarn-1461-7.patch, yarn-1461-8.patch
>
>




--
This message was sent by Atlassian JIRA
(v6.1.5#6160)

Reply via email to