[
https://issues.apache.org/jira/browse/YARN-1461?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13859025#comment-13859025
]
Sandy Ryza commented on YARN-1461:
----------------------------------
The patch is looking good. A few comments:
{code}
+ @Private
+ @Unstable
+ public abstract Set<String> getTags();
{code}
Getters in GetApplicationsRequest should be Public/Stable.
{code}
+
+
+ /**
+ * Get tags for the application
{code}
Two lines not needed here.
{code}
+ public abstract void setTags(Set<String> tags) throws
IllegalArgumentException;
{code}
IllegalArgumentException is a RuntimeException - we don't need a throws for it.
I think using a checked exception here would require unnecessary try/catch for
users who are doing it right. If they're using tags that violate the
constraints, they should be changing their code, not handling the exception.
{code}
+ Set<String> appTags = parseQueries(tags, false);
+ if (!appTags.isEmpty()) {
+ checkAppTags = true;
+ }
{code}
Null check?
{code}
+ appInfo.getTags()))).append("\",\"")
+ .append(StringEscapeUtils.escapeJavaScript(StringEscapeUtils.escapeHtml(
{code}
We need to add a column in the table header for this, right?
{code}
+ protected String tags = ""; // initialize to an empty string
{code}
Comment is unnecessary
{code}
+ if (app.getTags() != null && !app.getTags().isEmpty()) {
+ for (String tag : app.getTags()) {
+ this.tags += tag + ",";
+ }
+ this.tags = this.tags.substring(0, tags.length() - 1);
+ }
{code}
Use Joiner.on(",")
{code}
+
+ @Override
+ public Set<String> getTags() { return null; }
{code}
Use multiple lines as is done for other methods that return null in the class.
> 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
>
>
--
This message was sent by Atlassian JIRA
(v6.1.5#6160)
