Zhijie Shen commented on YARN-2569:

LGTM in general. Some comments about the patch.

1. Per discussion offline, is it a bit aggressive to mark the new APIs 
\@Stable? In particular when the class is marked \@Evolving. BTW, should we 
make LogAggregationContext \@Public?

2. It's good to describe what kind of pattern the user should use? Wildcard 

3. Miss a full stop?
+ *     how often the logAggregationSerivce uploads container logs in seconds

4. The description is broken?
+   *          to set

5. It shouldn't be part of API?
+  @Private
+  public abstract LogAggregationContextProto getProto();

> Log Handling for LRS API Changes
> --------------------------------
>                 Key: YARN-2569
>                 URL: https://issues.apache.org/jira/browse/YARN-2569
>             Project: Hadoop YARN
>          Issue Type: Sub-task
>          Components: nodemanager, resourcemanager
>            Reporter: Xuan Gong
>            Assignee: Xuan Gong
>         Attachments: YARN-2569.1.patch, YARN-2569.2.patch, YARN-2569.3.patch, 
> YARN-2569.4.1.patch, YARN-2569.4.patch

This message was sent by Atlassian JIRA

Reply via email to