Li Lu commented on YARN-3049:

Hi [~zjshen]! Some of my comments:

bq. As I see a lot of arguments for the reader interface (as well as the writer 
one) and the potential signature change in future (e.g, adding newApp in this 
patch), I start to think of grouping the primitive arguments, shielding them in 
some category object, such as EntityContext, EntityFilters, Opts and so on, and 
using these as the arguments of the interface instead. 

I agree. Actually I spent quite some time wondering if we really need to add 
the {{newApp}} argument in this patch. Encapsulating all related information 
into a category object appears to be a nice way to avoid future interface 
changes. +1. 

bq. Given it may be a non-trivial work, can we get this patch in and follow up 
the filter change in another jira just in case?

Definitely. Let's consolidate the whole workflow first. Then we can start these 

bq. In fact, it has been tested. I change the write path by letting newApp = 
true, and check if we can query the entity successfully without giving the 
flow/flowRun explicitly. However, I didn't do much assertion around the fields 
of retrieved entities, because I consider of deferring this work together with 
rewriting the whole HBase backend unit test.

Sounds good to me. 

> [Storage Implementation] Implement storage reader interface to fetch raw data 
> from HBase backend
> ------------------------------------------------------------------------------------------------
>                 Key: YARN-3049
>                 URL: https://issues.apache.org/jira/browse/YARN-3049
>             Project: Hadoop YARN
>          Issue Type: Sub-task
>          Components: timelineserver
>            Reporter: Sangjin Lee
>            Assignee: Zhijie Shen
>         Attachments: YARN-3049-WIP.1.patch, YARN-3049-WIP.2.patch, 
> YARN-3049-WIP.3.patch, YARN-3049-YARN-2928.2.patch, 
> YARN-3049-YARN-2928.3.patch
> Implement existing ATS queries with the new ATS reader design.

This message was sent by Atlassian JIRA

Reply via email to