[
https://issues.apache.org/jira/browse/YARN-4447?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15265425#comment-15265425
]
Varun Saxena commented on YARN-4447:
------------------------------------
bq. Overall, catching Exception}}s is too broad
Correct. Will tighten it.
bq. l.34: angle brackets in javadoc should be escaped
Ok.
bq. l.69: I would recommend using the Deque interface (and LinkedList) over
Stack, as Stack has unnecessary synchronization overhead
Thanks for the pointer. Yes, synchronization is not required here, so
ArrayDeque can be used here.
bq. l.75: this is where you can lowercase the expression
IMO, we cannot lowercase the whole expression as events/metrics/configs,etc.in
filters have to be matched as-is. Thoughts ?
bq. Does parseDataToRetrieve() merit its own TimelineParser() impl? It fits
with the rest of the parser classes
Hmm...We can have it inside a class as well. I am Ok with that.
bq. How much negative testing (e.g. invalid expressions) is covered? Do we need
to cover more of error cases?
I have covered a few negative cases in TestTimelineReaderWebServicesUtils. More
tests can be thought of and added. I have to add a few more test cases in the
patch above.
> Provide a mechanism to represent complex filters and parse them at the REST
> layer
> ----------------------------------------------------------------------------------
>
> Key: YARN-4447
> URL: https://issues.apache.org/jira/browse/YARN-4447
> Project: Hadoop YARN
> Issue Type: Sub-task
> Components: timelineserver
> Affects Versions: YARN-2928
> Reporter: Varun Saxena
> Assignee: Varun Saxena
> Labels: yarn-2928-1st-milestone
> Attachments: Timeline-Filters.pdf, YARN-4447-YARN-2928.01.patch
>
>
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
