[ 
https://issues.apache.org/jira/browse/YARN-1729?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13918507#comment-13918507
 ] 

Zhijie Shen commented on YARN-1729:
-----------------------------------

1. mapper is not necessary, objectReader and objectReader should be final, and 
both constants can be initiated in a static block. And please follow the name 
convention of the static final constant.
{code}
+  private static ObjectMapper mapper = new ObjectMapper();
+  private static ObjectReader objectReader = mapper.reader(Object.class);
+  private static ObjectWriter objectWriter = mapper.writer();
{code}

2. Similar problem here.
{code}
+  private static ObjectReader objectReader =
+      new ObjectMapper().reader(Object.class);
{code}

3. In the test case, would you mind adding one more test case of "other:123abc" 
to show the difference?
{code}
+    ClientResponse response = r.path("ws").path("v1").path("timeline")
+        .path("type_1").queryParam("primaryFilter", "other:\"123abc\"")
{code}

Other than that, the patch looks good to me.

In addition, I'm aware of an additional issue of the leveldb implementation, 
which is aware of JSON input specification. This means whenever our RESTful 
APIs allows to take XML input, the current implementation may not work 
correctly. IMHO, ideally the store should be isolated from the RESTful 
interface input types. Anyway, let's leave the issue separately not to block 
this patch, as the issue happens before this patch as well.

> TimelineWebServices always passes primary and secondary filters as strings
> --------------------------------------------------------------------------
>
>                 Key: YARN-1729
>                 URL: https://issues.apache.org/jira/browse/YARN-1729
>             Project: Hadoop YARN
>          Issue Type: Sub-task
>            Reporter: Billie Rinaldi
>            Assignee: Billie Rinaldi
>         Attachments: YARN-1729.1.patch, YARN-1729.2.patch, YARN-1729.3.patch, 
> YARN-1729.4.patch, YARN-1729.5.patch
>
>
> Primary filters and secondary filter values can be arbitrary json-compatible 
> Object.  The web services should determine if the filters specified as query 
> parameters are objects or strings before passing them to the store.



--
This message was sent by Atlassian JIRA
(v6.2#6252)

Reply via email to