Varun Saxena commented on YARN-3814:

Thanks for the review [~gtCarrera9].
Sorry hadn't looked at the reference patch.

bq. Also, it looks a little bit awkward to return a set of entities if we use 
an endpoint called "entity" (and vice versa).
Hmm...makes sense. Lets make it entity and entities then. That is semantically 
correct. Lets get opinion of others as well because this will be user facing 

bq. are there any special reasons we cannot adopt the way in the reference 
patch to parse the enum set?
Will change it.

bq. parseKeyStrValuesStr and parseKeyStrValueStr look pretty similar. 
Ok. Will make the change. May have to use generics here as maps are of 
different types.  

bq. Since we already have aggregation related designs in the reference patch, I 
suggest we include it in the final version of this patch. 
We have a JIRA for aggregation implementation for reader side. As we do not 
have parts of aggregation write path and we havent discussed timeline reader 
API for it, I think we can do it in the aggregation related JIRA. Because we 
were initially thinking of having different REST endpoints for different levels 
of aggregations. Thoughts ?

bq. FileSystemTimelineReaderImpl, why we need to swallow FileNotFoundExceptions?
The intention here is that if user queries for an entity which does not exist, 
the entity file would not exist. FileNotFoundException has been swallowed to 
return null which will then be converted to HTTP 404 in 

> 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.reference.patch

This message was sent by Atlassian JIRA

Reply via email to