Re: Review Request 52119: LENS-1335 NullPointerException in LensClient::getResults(QueryHandle query)

[Srikanth Sundarrajan]

> On Sept. 22, 2016, 5:56 a.m., Amareshwari Sriramadasu wrote:
> > lens-client/src/main/java/org/apache/lens/client/LensClient.java, line 330
> > 
> >
> > We would need an api to getResults for particular handle as well - to 
> > access it across clients, no? I see current caching may not work. But if 
> > not cached, client should fetch it from srver.
> 
> Srikanth Sundarrajan wrote:
> Upon execution of a new query in lens client, a new statement object 
> isn't created, so essentially, the statement object holds only LensQuery 
> handle of last executed query. The current assumption in 
> getLensStatement(queryHandle) will return query handle specific statement is 
> broken. If there is merit in caching LensQuery objects of every query 
> executed by a client/statement, then this requires a far deeper fix. I can 
> change the scope of the issue and revise the patch accordingly, but we need 
> to be convinced about the need to cache the LensQuery object. It seems 
> unncessary to cache a LensQuery object particuarly if it is unfinished, as 
> the state might change as the query makes progress. The caching can happen in 
> the consuming application in such case given that the LensQuery for finished 
> queries are immutable. Would like you hear your views before proceeding 
> further.
> 
> Rajat Khandelwal wrote:
> The query shouldn't be cached unless it's in a terminal state. I have 
> incorporated this kind of caching in the python client.
> 
> Srikanth Sundarrajan wrote:
> Agreed. Caching unfinished LensQuery object doesn't seem like the right 
> thing to do, will wait for @Amareshwari also to chime in (in case am missing 
> something), and then we can move further on this.
> 
> Amareshwari Sriramadasu wrote:
> I totally agree, caching for LensQuery object is not required. I think 
> current caching is there for cli command to accept commands without query 
> handle to get details of last executed query.
> 
> For the problem in the issue - getResults for a handle - client can 
> always get results from server for the handle.

Thanks Amareshwari & Rajat, Are there any changes suggested/required on this 
patch ?


- Srikanth


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


On Sept. 21, 2016, 2:14 p.m., Srikanth Sundarrajan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52119/
> ---
> 
> (Updated Sept. 21, 2016, 2:14 p.m.)
> 
> 
> Review request for lens.
> 
> 
> Bugs: LENS-1335
> https://issues.apache.org/jira/browse/LENS-1335
> 
> 
> Repository: lens
> 
> 
> Description
> ---
> 
> NullPointerException in LensClient::getResults(QueryHandle query)
> 
> 
> Diffs
> -
> 
>   lens-cli/src/test/java/org/apache/lens/cli/TestLensQueryCommands.java 
> e75fc0e 
>   lens-client/src/main/java/org/apache/lens/client/LensClient.java 593cc08 
> 
> Diff: https://reviews.apache.org/r/52119/diff/
> 
> 
> Testing
> ---
> 
> Modified tests to avoid using offending method.
> 
> 
> Thanks,
> 
> Srikanth Sundarrajan
> 
>

Re: Review Request 52119: LENS-1335 NullPointerException in LensClient::getResults(QueryHandle query)

[Amareshwari Sriramadasu]

> On Sept. 22, 2016, 5:56 a.m., Amareshwari Sriramadasu wrote:
> > lens-client/src/main/java/org/apache/lens/client/LensClient.java, line 330
> > 
> >
> > We would need an api to getResults for particular handle as well - to 
> > access it across clients, no? I see current caching may not work. But if 
> > not cached, client should fetch it from srver.
> 
> Srikanth Sundarrajan wrote:
> Upon execution of a new query in lens client, a new statement object 
> isn't created, so essentially, the statement object holds only LensQuery 
> handle of last executed query. The current assumption in 
> getLensStatement(queryHandle) will return query handle specific statement is 
> broken. If there is merit in caching LensQuery objects of every query 
> executed by a client/statement, then this requires a far deeper fix. I can 
> change the scope of the issue and revise the patch accordingly, but we need 
> to be convinced about the need to cache the LensQuery object. It seems 
> unncessary to cache a LensQuery object particuarly if it is unfinished, as 
> the state might change as the query makes progress. The caching can happen in 
> the consuming application in such case given that the LensQuery for finished 
> queries are immutable. Would like you hear your views before proceeding 
> further.
> 
> Rajat Khandelwal wrote:
> The query shouldn't be cached unless it's in a terminal state. I have 
> incorporated this kind of caching in the python client.
> 
> Srikanth Sundarrajan wrote:
> Agreed. Caching unfinished LensQuery object doesn't seem like the right 
> thing to do, will wait for @Amareshwari also to chime in (in case am missing 
> something), and then we can move further on this.

I totally agree, caching for LensQuery object is not required. I think current 
caching is there for cli command to accept commands without query handle to get 
details of last executed query.

For the problem in the issue - getResults for a handle - client can always get 
results from server for the handle.


- Amareshwari


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


On Sept. 21, 2016, 2:14 p.m., Srikanth Sundarrajan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52119/
> ---
> 
> (Updated Sept. 21, 2016, 2:14 p.m.)
> 
> 
> Review request for lens.
> 
> 
> Bugs: LENS-1335
> https://issues.apache.org/jira/browse/LENS-1335
> 
> 
> Repository: lens
> 
> 
> Description
> ---
> 
> NullPointerException in LensClient::getResults(QueryHandle query)
> 
> 
> Diffs
> -
> 
>   lens-cli/src/test/java/org/apache/lens/cli/TestLensQueryCommands.java 
> e75fc0e 
>   lens-client/src/main/java/org/apache/lens/client/LensClient.java 593cc08 
> 
> Diff: https://reviews.apache.org/r/52119/diff/
> 
> 
> Testing
> ---
> 
> Modified tests to avoid using offending method.
> 
> 
> Thanks,
> 
> Srikanth Sundarrajan
> 
>

Re: Review Request 52119: LENS-1335 NullPointerException in LensClient::getResults(QueryHandle query)

[Srikanth Sundarrajan]

> On Sept. 22, 2016, 5:56 a.m., Amareshwari Sriramadasu wrote:
> > lens-client/src/main/java/org/apache/lens/client/LensClient.java, line 330
> > 
> >
> > We would need an api to getResults for particular handle as well - to 
> > access it across clients, no? I see current caching may not work. But if 
> > not cached, client should fetch it from srver.
> 
> Srikanth Sundarrajan wrote:
> Upon execution of a new query in lens client, a new statement object 
> isn't created, so essentially, the statement object holds only LensQuery 
> handle of last executed query. The current assumption in 
> getLensStatement(queryHandle) will return query handle specific statement is 
> broken. If there is merit in caching LensQuery objects of every query 
> executed by a client/statement, then this requires a far deeper fix. I can 
> change the scope of the issue and revise the patch accordingly, but we need 
> to be convinced about the need to cache the LensQuery object. It seems 
> unncessary to cache a LensQuery object particuarly if it is unfinished, as 
> the state might change as the query makes progress. The caching can happen in 
> the consuming application in such case given that the LensQuery for finished 
> queries are immutable. Would like you hear your views before proceeding 
> further.
> 
> Rajat Khandelwal wrote:
> The query shouldn't be cached unless it's in a terminal state. I have 
> incorporated this kind of caching in the python client.

Agreed. Caching unfinished LensQuery object doesn't seem like the right thing 
to do, will wait for @Amareshwari also to chime in (in case am missing 
something), and then we can move further on this.


- Srikanth


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


On Sept. 21, 2016, 2:14 p.m., Srikanth Sundarrajan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52119/
> ---
> 
> (Updated Sept. 21, 2016, 2:14 p.m.)
> 
> 
> Review request for lens.
> 
> 
> Bugs: LENS-1335
> https://issues.apache.org/jira/browse/LENS-1335
> 
> 
> Repository: lens
> 
> 
> Description
> ---
> 
> NullPointerException in LensClient::getResults(QueryHandle query)
> 
> 
> Diffs
> -
> 
>   lens-cli/src/test/java/org/apache/lens/cli/TestLensQueryCommands.java 
> e75fc0e 
>   lens-client/src/main/java/org/apache/lens/client/LensClient.java 593cc08 
> 
> Diff: https://reviews.apache.org/r/52119/diff/
> 
> 
> Testing
> ---
> 
> Modified tests to avoid using offending method.
> 
> 
> Thanks,
> 
> Srikanth Sundarrajan
> 
>

Re: Review Request 52119: LENS-1335 NullPointerException in LensClient::getResults(QueryHandle query)

[Rajat Khandelwal]

> On Sept. 22, 2016, 11:26 a.m., Amareshwari Sriramadasu wrote:
> > lens-client/src/main/java/org/apache/lens/client/LensClient.java, line 330
> > 
> >
> > We would need an api to getResults for particular handle as well - to 
> > access it across clients, no? I see current caching may not work. But if 
> > not cached, client should fetch it from srver.
> 
> Srikanth Sundarrajan wrote:
> Upon execution of a new query in lens client, a new statement object 
> isn't created, so essentially, the statement object holds only LensQuery 
> handle of last executed query. The current assumption in 
> getLensStatement(queryHandle) will return query handle specific statement is 
> broken. If there is merit in caching LensQuery objects of every query 
> executed by a client/statement, then this requires a far deeper fix. I can 
> change the scope of the issue and revise the patch accordingly, but we need 
> to be convinced about the need to cache the LensQuery object. It seems 
> unncessary to cache a LensQuery object particuarly if it is unfinished, as 
> the state might change as the query makes progress. The caching can happen in 
> the consuming application in such case given that the LensQuery for finished 
> queries are immutable. Would like you hear your views before proceeding 
> further.

The query shouldn't be cached unless it's in a terminal state. I have 
incorporated this kind of caching in the python client.


- Rajat


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


On Sept. 21, 2016, 7:44 p.m., Srikanth Sundarrajan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52119/
> ---
> 
> (Updated Sept. 21, 2016, 7:44 p.m.)
> 
> 
> Review request for lens.
> 
> 
> Bugs: LENS-1335
> https://issues.apache.org/jira/browse/LENS-1335
> 
> 
> Repository: lens
> 
> 
> Description
> ---
> 
> NullPointerException in LensClient::getResults(QueryHandle query)
> 
> 
> Diffs
> -
> 
>   lens-cli/src/test/java/org/apache/lens/cli/TestLensQueryCommands.java 
> e75fc0e 
>   lens-client/src/main/java/org/apache/lens/client/LensClient.java 593cc08 
> 
> Diff: https://reviews.apache.org/r/52119/diff/
> 
> 
> Testing
> ---
> 
> Modified tests to avoid using offending method.
> 
> 
> Thanks,
> 
> Srikanth Sundarrajan
> 
>

Re: Review Request 52119: LENS-1335 NullPointerException in LensClient::getResults(QueryHandle query)

[Srikanth Sundarrajan]

> On Sept. 22, 2016, 5:56 a.m., Amareshwari Sriramadasu wrote:
> > lens-client/src/main/java/org/apache/lens/client/LensClient.java, line 330
> > 
> >
> > We would need an api to getResults for particular handle as well - to 
> > access it across clients, no? I see current caching may not work. But if 
> > not cached, client should fetch it from srver.

Upon execution of a new query in lens client, a new statement object isn't 
created, so essentially, the statement object holds only LensQuery handle of 
last executed query. The current assumption in getLensStatement(queryHandle) 
will return query handle specific statement is broken. If there is merit in 
caching LensQuery objects of every query executed by a client/statement, then 
this requires a far deeper fix. I can change the scope of the issue and revise 
the patch accordingly, but we need to be convinced about the need to cache the 
LensQuery object. It seems unncessary to cache a LensQuery object particuarly 
if it is unfinished, as the state might change as the query makes progress. The 
caching can happen in the consuming application in such case given that the 
LensQuery for finished queries are immutable. Would like you hear your views 
before proceeding further.


- Srikanth


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


On Sept. 21, 2016, 2:14 p.m., Srikanth Sundarrajan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52119/
> ---
> 
> (Updated Sept. 21, 2016, 2:14 p.m.)
> 
> 
> Review request for lens.
> 
> 
> Bugs: LENS-1335
> https://issues.apache.org/jira/browse/LENS-1335
> 
> 
> Repository: lens
> 
> 
> Description
> ---
> 
> NullPointerException in LensClient::getResults(QueryHandle query)
> 
> 
> Diffs
> -
> 
>   lens-cli/src/test/java/org/apache/lens/cli/TestLensQueryCommands.java 
> e75fc0e 
>   lens-client/src/main/java/org/apache/lens/client/LensClient.java 593cc08 
> 
> Diff: https://reviews.apache.org/r/52119/diff/
> 
> 
> Testing
> ---
> 
> Modified tests to avoid using offending method.
> 
> 
> Thanks,
> 
> Srikanth Sundarrajan
> 
>

Re: Review Request 52119: LENS-1335 NullPointerException in LensClient::getResults(QueryHandle query)

[Amareshwari Sriramadasu]

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




lens-client/src/main/java/org/apache/lens/client/LensClient.java (line 330)


We would need an api to getResults for particular handle as well - to 
access it across clients, no? I see current caching may not work. But if not 
cached, client should fetch it from srver.


- Amareshwari Sriramadasu


On Sept. 21, 2016, 2:14 p.m., Srikanth Sundarrajan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52119/
> ---
> 
> (Updated Sept. 21, 2016, 2:14 p.m.)
> 
> 
> Review request for lens.
> 
> 
> Bugs: LENS-1335
> https://issues.apache.org/jira/browse/LENS-1335
> 
> 
> Repository: lens
> 
> 
> Description
> ---
> 
> NullPointerException in LensClient::getResults(QueryHandle query)
> 
> 
> Diffs
> -
> 
>   lens-cli/src/test/java/org/apache/lens/cli/TestLensQueryCommands.java 
> e75fc0e 
>   lens-client/src/main/java/org/apache/lens/client/LensClient.java 593cc08 
> 
> Diff: https://reviews.apache.org/r/52119/diff/
> 
> 
> Testing
> ---
> 
> Modified tests to avoid using offending method.
> 
> 
> Thanks,
> 
> Srikanth Sundarrajan
> 
>

Review Request 52119: LENS-1335 NullPointerException in LensClient::getResults(QueryHandle query)

[Srikanth Sundarrajan]

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

Review request for lens.


Bugs: LENS-1335
https://issues.apache.org/jira/browse/LENS-1335


Repository: lens


Description
---

NullPointerException in LensClient::getResults(QueryHandle query)


Diffs
-

  lens-cli/src/test/java/org/apache/lens/cli/TestLensQueryCommands.java e75fc0e 
  lens-client/src/main/java/org/apache/lens/client/LensClient.java 593cc08 

Diff: https://reviews.apache.org/r/52119/diff/


Testing
---

Modified tests to avoid using offending method.


Thanks,

Srikanth Sundarrajan