[
https://issues.apache.org/jira/browse/YARN-4224?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15116331#comment-15116331
]
Sangjin Lee commented on YARN-4224:
-----------------------------------
The patch looks mostly good. I only have a few minor comments.
(TimelineContext.java)
- l.29: nit: let's use the primitive {{long}} as opposed to object {{Long}}
- l.99: same
- l.131: same
(TimelineCollectorContext.java)
- l.35: let's use the primitive {{long}}
- l.57-59: redundant as the super class checks the same condition
(TimelineReaderContext.java)
- l.56-58: redundant as the super class checks the same condition
(TimelineUIDConverter.java)
- l.26: I don't see this class ever needed outside the reader package; can we
make it a package-private class instead of a public class?
- l.197, 203: this might be more about formality, but I think we may want to
make these public because these are really part of the "public" contract
(TimelineReaderWebServices.java)
- l.322: we shouldn't use Throwable.printStackTrace() which goes to standard
err console; we should use the logger
(TimelineReaderUtils.java)
- l.41: the same for this class; can we make this a package-private class? As a
rule, if you think it is safe to assume that some class should never be used
outside the reader package, it should be made package-private to reduce the
public API
> Support fetching entities by UID and change the REST interface to conform to
> current REST APIs' in YARN
> -------------------------------------------------------------------------------------------------------
>
> Key: YARN-4224
> URL: https://issues.apache.org/jira/browse/YARN-4224
> Project: Hadoop YARN
> Issue Type: Sub-task
> Components: timelineserver
> Affects Versions: YARN-2928
> Reporter: Varun Saxena
> Assignee: Varun Saxena
> Labels: yarn-2928-1st-milestone
> Attachments: YARN-4224-YARN-2928.01.patch,
> YARN-4224-YARN-2928.05.patch, YARN-4224-feature-YARN-2928.04.patch,
> YARN-4224-feature-YARN-2928.05.patch,
> YARN-4224-feature-YARN-2928.wip.02.patch,
> YARN-4224-feature-YARN-2928.wip.03.patch
>
>
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
