[
https://issues.apache.org/jira/browse/YARN-4446?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15126988#comment-15126988
]
Sangjin Lee commented on YARN-4446:
-----------------------------------
Thanks [~varun_saxena] for your patch!
I generally agree with the direction you are taking with this patch. I think
the rule of thumb is, the impact of any changes to filters or data to retrieve
should be limited to {{TimelineReaderWebServices}} and {{\*EntityReaders}}, and
should not result in changes in any other place. I think that is the case with
the patch, but can you also confirm?
Regarding *common* default behavior we have throughout the code base, should we
move them to {{TimelineEntityFilters}} and {{TimelineDataToRetrieve}}
respectively and encapsulate them? The examples include things like
DEFAULT_LIMIT and behavior around default data to retrieve if the user didn't
specify any.
(TimelineContext.java)
- nit: it might be slightly better to call the other constructor
(TimelineReaderContext.java)
- nit: same as TimelineContext
(TimelineReaderManager.java)
- l.127: why is this copy necessary?
(TimelineReader.java)
- The javadoc that's added here is great. My only question is, where should we
have the detailed description of the filters and data to retrieve? Should we
put them in TimelineDataToRetrieve and TimelineEntityFilters instead?
> Refactor reader API for better extensibility
> --------------------------------------------
>
> Key: YARN-4446
> URL: https://issues.apache.org/jira/browse/YARN-4446
> 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-4446-YARN-2928.01.patch,
> YARN-4446-YARN-2928.02.patch
>
>
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
