hadoop-yarn-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Li Lu (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (YARN-3814) REST API implementation for getting raw entities in TimelineReader
Date Tue, 11 Aug 2015 00:47:46 GMT

    [ 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)

Mime
View raw message