> On Sept. 2, 2016, 11:44 a.m., Amareshwari Sriramadasu wrote:
> > lens-ui/app/components/QueryResultsComponent.js, line 91
> > <https://reviews.apache.org/r/51552/diff/2/?file=1489049#file1489049line91>
> >
> >     commented code?

Yeah, sorting based on time can't be done unless data for all queries is 
fetched. So I removed client side sorting. Now sorting is server side. The 
order the server sends handles in, is the order they are displayed here. Server 
can further modify the code to send data sorted by submnission time.


> On Sept. 2, 2016, 11:44 a.m., Amareshwari Sriramadasu wrote:
> > lens-ui/app/components/QueryOperationsComponent.js, line 59
> > <https://reviews.apache.org/r/51552/diff/2/?file=1489047#file1489047line59>
> >
> >     fromDate should be now, and endDate to now.day - 2 days, no ?
> >     
> >     Also, I feel no time filter should be passed for running or queued 
> > queries. Lets pass time filter only for completed queries.

Yeah, no time filter is passed for running and queued pages by default. But in 
other pages, time filter is provided. The filtering logic in server is fromTime 
<= submissionTime < endTime. So the filter is correct.


> On Sept. 2, 2016, 11:44 a.m., Amareshwari Sriramadasu wrote:
> > lens-ui/app/components/QueryResultsComponent.js, line 145
> > <https://reviews.apache.org/r/51552/diff/2/?file=1489049#file1489049line145>
> >
> >     Seems some debugging code here

So there is an architecture change here. Earlier, in each page, it was getting 
all query details. Which is two steps: get handles with the state filter, then 
make subsequent get calls for getting details of all handles. Now, with ui 
having added capability of more filters, this would become overwhelmingly slow, 
since a user can want to see failed queries of last 2 days, then last 10 days, 
then last 2 days again. Every change would require a large number of calls. So 
a caching layer is added, which ensures that queries are only fetched when the 
details are not already fetched. There needs to be a further optimization to 
keep fetching status of RUNNING/QUEUED queries, but will take that up later. 
Now, in case, a query's details are already there, it's used, else a dummy 
query object is used. So I picked a query object and pasted that here, changed 
the values to indicate the "loading" status. Then some fields were unused in 
the rendering logic so I commented those fields. But didn't rem
 ove the comments as it seemed like a good idea to keep it as comment for 
readability.


- Rajat


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/51552/#review147626
-----------------------------------------------------------


On Aug. 31, 2016, 12:44 p.m., Rajat Khandelwal wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51552/
> -----------------------------------------------------------
> 
> (Updated Aug. 31, 2016, 12:44 p.m.)
> 
> 
> Review request for lens.
> 
> 
> Bugs: LENS-1303
>     https://issues.apache.org/jira/browse/LENS-1303
> 
> 
> Repository: lens
> 
> 
> Description
> -------
> 
> 
> Diffs
> -----
> 
>   lens-ui/app/actions/AdhocQueryActions.js 
> 38f2794a443c7814a20a9662efb06c9b431e764b 
>   lens-ui/app/adapters/AdhocQueryAdapter.js 
> a54274fbf1e83d06ca7175eb9c3905662731067a 
>   lens-ui/app/components/CubeSchemaComponent.js 
> 9c23b9fd3cd8a6ae87802ce9a07f2267ef76f8f7 
>   lens-ui/app/components/QueryOperationsComponent.js 
> e4cc1e7304092ea6fcce28d6ed3d895f5061d982 
>   lens-ui/app/components/QueryPreviewComponent.js 
> a29f2d8ad71c59cbc7ba22ac7e95512399acc16f 
>   lens-ui/app/components/QueryResultsComponent.js 
> 01f0e30415bd2f2a7d2acfdddf51c388fc683ed8 
>   lens-ui/app/constants/AdhocQueryConstants.js 
> 7eceb6fec6ca64497bbc78973ecb818e2bd89190 
>   lens-ui/app/stores/AdhocQueryStore.js 
> d8891c26e5003b09c512db9352841a654fdde27a 
> 
> Diff: https://reviews.apache.org/r/51552/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Rajat Khandelwal
> 
>

Reply via email to