[
https://issues.apache.org/jira/browse/YARN-3814?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14681029#comment-14681029
]
Li Lu commented on YARN-3814:
-----------------------------
Hi [~varun_saxena], thanks for the work. I've gone through the latest patch and
in general it's quite close. Some of my follow up comments:
In TimelineReaderWebServices:
{code}
+ try {
+ Object value =
+ GenericObjectMapper.OBJECT_READER.readValue(pairStrs[1].trim());
+ map.put(pairStrs[0].trim(), (T) value);
+ } catch (IOException e) {
+ map.put(pairStrs[0].trim(), (T) pairStrs[1].trim());
+ }
{code}
Why we're not using if statement but using exception handling to decide the
type of pairStrs[1]? I think we can pretty much restrict the type of
pairStrs[1] here, so maybe instanceof will do the work?
I suggest move parseKeyValue to somewhere else so that we can put all "user
facing" parse methods together. Meanwhile, are there any hopes to let
parseKeyStrValuesStr reuse parseKeyValue? The code looks pretty similar and we
can probably reuse parseKeyValue as well. We may further want to move those
parse methods into a helper method collection file?
In TestTimelineReaderWebServices:
private static TimelineEntity entity(String type, String id)
Change its name into newEntity or initEntity?
l.125 String msg = new String(); instead of using immediate value?
It's great that in this patch there are sufficient unit test cases. This is
awesome. However, we need to be careful with the long URLs with fixed
delimiters.
> REST API implementation for getting raw entities in TimelineReader
> ------------------------------------------------------------------
>
> Key: YARN-3814
> URL: https://issues.apache.org/jira/browse/YARN-3814
> Project: Hadoop YARN
> Issue Type: Sub-task
> Components: timelineserver
> Affects Versions: YARN-2928
> Reporter: Varun Saxena
> Assignee: Varun Saxena
> Attachments: YARN-3814-YARN-2928.01.patch,
> YARN-3814-YARN-2928.02.patch, YARN-3814-YARN-2928.03.patch,
> YARN-3814-YARN-2928.04.patch, YARN-3814.reference.patch
>
>
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
