You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@lens.apache.org by Rajat Khandelwal <ra...@gmail.com> on 2016/05/09 14:43:30 UTC

Review Request 47121: LENS-1010: Make LensClient implement java.lang.Closable

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

Review request for lens.


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


Repository: lens


Description
-------

This would allow users to use lens client in try-with-resources clause without worrying about closing manually.

https://docs.oracle.com/javase/tutorial/essential/exceptions/tryResourceClose.html


Diffs
-----

  lens-client/src/main/java/org/apache/lens/client/LensClient.java 43da691aff52bc108f99033fac2e8e068ae1c216 
  lens-client/src/main/java/org/apache/lens/client/LensConnection.java 49518662eb17c1fd24deb26ddd46a36c8826bc5b 

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


Testing
-------


Thanks,

Rajat Khandelwal


Re: Review Request 47121: LENS-1010: Make LensClient implement java.lang.Closable

Posted by Rajat Khandelwal <ra...@gmail.com>.

> On May 10, 2016, 8:22 a.m., Puneet Gupta wrote:
> > lens-client/src/main/java/org/apache/lens/client/LensConnection.java, line 214
> > <https://reviews.apache.org/r/47121/diff/2/?file=1376577#file1376577line214>
> >
> >     should we have only one public close*() method ? Two methods can be confusing for end user.
> >     If we need to return APIResult, we can not adhere to Closeable Interface

So there are two approaches, each with trade-offs

1. Change the return type to void. This makes sense, since close should just be closing everything without the client needing to take care of the return output. This is what I did in revision 1. The down-side is that existing code that's using this method and depending on return type will become incompatible. 
2. Have two close method, as done in revision 2. More confusing, still makes existing clients to change. 

I'm also inclined towards approach 1, let me know your thoughts.


- Rajat


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


On May 9, 2016, 8:48 p.m., Rajat Khandelwal wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47121/
> -----------------------------------------------------------
> 
> (Updated May 9, 2016, 8:48 p.m.)
> 
> 
> Review request for lens.
> 
> 
> Bugs: LENS-1010
>     https://issues.apache.org/jira/browse/LENS-1010
> 
> 
> Repository: lens
> 
> 
> Description
> -------
> 
> This would allow users to use lens client in try-with-resources clause without worrying about closing manually.
> 
> https://docs.oracle.com/javase/tutorial/essential/exceptions/tryResourceClose.html
> 
> 
> Diffs
> -----
> 
>   lens-client/src/main/java/org/apache/lens/client/LensClient.java 43da691aff52bc108f99033fac2e8e068ae1c216 
>   lens-client/src/main/java/org/apache/lens/client/LensConnection.java 49518662eb17c1fd24deb26ddd46a36c8826bc5b 
> 
> Diff: https://reviews.apache.org/r/47121/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Rajat Khandelwal
> 
>


Re: Review Request 47121: LENS-1010: Make LensClient implement java.lang.Closable

Posted by Puneet Gupta <pu...@gmail.com>.

> On May 10, 2016, 2:52 a.m., Puneet Gupta wrote:
> > lens-client/src/main/java/org/apache/lens/client/LensConnection.java, line 214
> > <https://reviews.apache.org/r/47121/diff/2/?file=1376577#file1376577line214>
> >
> >     should we have only one public close*() method ? Two methods can be confusing for end user.
> >     If we need to return APIResult, we can not adhere to Closeable Interface
> 
> Rajat Khandelwal wrote:
>     So there are two approaches, each with trade-offs
>     
>     1. Change the return type to void. This makes sense, since close should just be closing everything without the client needing to take care of the return output. This is what I did in revision 1. The down-side is that existing code that's using this method and depending on return type will become incompatible. 
>     2. Have two close method, as done in revision 2. More confusing, still makes existing clients to change. 
>     
>     I'm also inclined towards approach 1, let me know your thoughts.

+1 for approach 1 . Anyways we have made incompatible changes to LensStatement also. We can add this to set of improvements. User should not worry after calling close method. Hoping that existing users are already not validating output of close


- Puneet


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


On May 9, 2016, 3:18 p.m., Rajat Khandelwal wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47121/
> -----------------------------------------------------------
> 
> (Updated May 9, 2016, 3:18 p.m.)
> 
> 
> Review request for lens.
> 
> 
> Bugs: LENS-1010
>     https://issues.apache.org/jira/browse/LENS-1010
> 
> 
> Repository: lens
> 
> 
> Description
> -------
> 
> This would allow users to use lens client in try-with-resources clause without worrying about closing manually.
> 
> https://docs.oracle.com/javase/tutorial/essential/exceptions/tryResourceClose.html
> 
> 
> Diffs
> -----
> 
>   lens-client/src/main/java/org/apache/lens/client/LensClient.java 43da691aff52bc108f99033fac2e8e068ae1c216 
>   lens-client/src/main/java/org/apache/lens/client/LensConnection.java 49518662eb17c1fd24deb26ddd46a36c8826bc5b 
> 
> Diff: https://reviews.apache.org/r/47121/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Rajat Khandelwal
> 
>


Re: Review Request 47121: LENS-1010: Make LensClient implement java.lang.Closable

Posted by Puneet Gupta <pu...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/47121/#review132365
-----------------------------------------------------------




lens-client/src/main/java/org/apache/lens/client/LensConnection.java (line 214)
<https://reviews.apache.org/r/47121/#comment196546>

    should we have only one public close*() method ? Two methods can be confusing for end user.
    If we need to return APIResult, we can not adhere to Closeable Interface


- Puneet Gupta


On May 9, 2016, 3:18 p.m., Rajat Khandelwal wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47121/
> -----------------------------------------------------------
> 
> (Updated May 9, 2016, 3:18 p.m.)
> 
> 
> Review request for lens.
> 
> 
> Bugs: LENS-1010
>     https://issues.apache.org/jira/browse/LENS-1010
> 
> 
> Repository: lens
> 
> 
> Description
> -------
> 
> This would allow users to use lens client in try-with-resources clause without worrying about closing manually.
> 
> https://docs.oracle.com/javase/tutorial/essential/exceptions/tryResourceClose.html
> 
> 
> Diffs
> -----
> 
>   lens-client/src/main/java/org/apache/lens/client/LensClient.java 43da691aff52bc108f99033fac2e8e068ae1c216 
>   lens-client/src/main/java/org/apache/lens/client/LensConnection.java 49518662eb17c1fd24deb26ddd46a36c8826bc5b 
> 
> Diff: https://reviews.apache.org/r/47121/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Rajat Khandelwal
> 
>


Re: Review Request 47121: LENS-1010: Make LensClient implement java.lang.Closable

Posted by Rajat Khandelwal <ra...@gmail.com>.

> On May 12, 2016, 8:33 a.m., Puneet Gupta wrote:
> > lens-client/src/main/java/org/apache/lens/client/LensConnection.java, line 230
> > <https://reviews.apache.org/r/47121/diff/3/?file=1377946#file1377946line230>
> >
> >     When are we expecting this exception?

This came once in test cases when the http call itself failed. Hence added some retry mechanism.


> On May 12, 2016, 8:33 a.m., Puneet Gupta wrote:
> > lens-client/src/main/java/org/apache/lens/client/LensConnection.java, line 240
> > <https://reviews.apache.org/r/47121/diff/3/?file=1377946#file1377946line240>
> >
> >     Will this code flow be reached or will it stop at if (processingException != null)  instead in this case ?

It should stop at the earlier condition, but as per my IDE, response might still not have been initialized, and the further lines are prone to NPE (:P), so added one more check as fool-proofing.


- Rajat


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


On May 10, 2016, 6:16 p.m., Rajat Khandelwal wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47121/
> -----------------------------------------------------------
> 
> (Updated May 10, 2016, 6:16 p.m.)
> 
> 
> Review request for lens.
> 
> 
> Bugs: LENS-1010
>     https://issues.apache.org/jira/browse/LENS-1010
> 
> 
> Repository: lens
> 
> 
> Description
> -------
> 
> This would allow users to use lens client in try-with-resources clause without worrying about closing manually.
> 
> https://docs.oracle.com/javase/tutorial/essential/exceptions/tryResourceClose.html
> 
> 
> Diffs
> -----
> 
>   lens-client/src/main/java/org/apache/lens/client/LensClient.java 43da691aff52bc108f99033fac2e8e068ae1c216 
>   lens-client/src/main/java/org/apache/lens/client/LensConnection.java 49518662eb17c1fd24deb26ddd46a36c8826bc5b 
>   lens-client/src/test/java/org/apache/lens/client/TestLensClient.java ff3b888038b44dedeb137853c541fb1bb68cc0ef 
>   lens-server/src/main/java/org/apache/lens/server/BaseLensService.java 9f8ee7251789cd943e7c6a1091feb44684f248c8 
>   lens-server/src/main/java/org/apache/lens/server/LensServices.java 46bae242bb627b1b542dd37c05baa2b788bd78ed 
>   lens-server/src/main/java/org/apache/lens/server/metastore/MetastoreResource.java 7395c832b8cbe6ccde811a8b76fc2e7f97d18c4e 
> 
> Diff: https://reviews.apache.org/r/47121/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Rajat Khandelwal
> 
>


Re: Review Request 47121: LENS-1010: Make LensClient implement java.lang.Closable

Posted by Puneet Gupta <pu...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/47121/#review132808
-----------------------------------------------------------




lens-client/src/main/java/org/apache/lens/client/LensConnection.java (line 215)
<https://reviews.apache.org/r/47121/#comment197060>

    we may need to synchronize this method if multiple closes are to be prevented?



lens-client/src/main/java/org/apache/lens/client/LensConnection.java (line 228)
<https://reviews.apache.org/r/47121/#comment197058>

    When are we expecting this exception?



lens-client/src/main/java/org/apache/lens/client/LensConnection.java (line 236)
<https://reviews.apache.org/r/47121/#comment197059>

    Will this code flow be reached or will it stop at if (processingException != null)  instead in this case ?


- Puneet Gupta


On May 10, 2016, 12:46 p.m., Rajat Khandelwal wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47121/
> -----------------------------------------------------------
> 
> (Updated May 10, 2016, 12:46 p.m.)
> 
> 
> Review request for lens.
> 
> 
> Bugs: LENS-1010
>     https://issues.apache.org/jira/browse/LENS-1010
> 
> 
> Repository: lens
> 
> 
> Description
> -------
> 
> This would allow users to use lens client in try-with-resources clause without worrying about closing manually.
> 
> https://docs.oracle.com/javase/tutorial/essential/exceptions/tryResourceClose.html
> 
> 
> Diffs
> -----
> 
>   lens-client/src/main/java/org/apache/lens/client/LensClient.java 43da691aff52bc108f99033fac2e8e068ae1c216 
>   lens-client/src/main/java/org/apache/lens/client/LensConnection.java 49518662eb17c1fd24deb26ddd46a36c8826bc5b 
>   lens-client/src/test/java/org/apache/lens/client/TestLensClient.java ff3b888038b44dedeb137853c541fb1bb68cc0ef 
>   lens-server/src/main/java/org/apache/lens/server/BaseLensService.java 9f8ee7251789cd943e7c6a1091feb44684f248c8 
>   lens-server/src/main/java/org/apache/lens/server/LensServices.java 46bae242bb627b1b542dd37c05baa2b788bd78ed 
>   lens-server/src/main/java/org/apache/lens/server/metastore/MetastoreResource.java 7395c832b8cbe6ccde811a8b76fc2e7f97d18c4e 
> 
> Diff: https://reviews.apache.org/r/47121/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Rajat Khandelwal
> 
>


Re: Review Request 47121: LENS-1010: Make LensClient implement java.lang.Closable

Posted by Amareshwari Sriramadasu <am...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/47121/#review134522
-----------------------------------------------------------




src/site/apt/user/olap-query-conf.apt 
<https://reviews.apache.org/r/47121/#comment199392>

    Seems was not added in olap-query-conf.xml and only apt update was done. Can we correct it?


- Amareshwari Sriramadasu


On May 21, 2016, 8:33 a.m., Rajat Khandelwal wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47121/
> -----------------------------------------------------------
> 
> (Updated May 21, 2016, 8:33 a.m.)
> 
> 
> Review request for lens.
> 
> 
> Bugs: LENS-1010
>     https://issues.apache.org/jira/browse/LENS-1010
> 
> 
> Repository: lens
> 
> 
> Description
> -------
> 
> This would allow users to use lens client in try-with-resources clause without worrying about closing manually.
> 
> https://docs.oracle.com/javase/tutorial/essential/exceptions/tryResourceClose.html
> 
> 
> Diffs
> -----
> 
>   lens-client/src/main/java/org/apache/lens/client/LensClient.java 480b6f8e5b7d98f10e5315beb41955597acbd4c3 
>   lens-client/src/main/java/org/apache/lens/client/LensConnection.java 49518662eb17c1fd24deb26ddd46a36c8826bc5b 
>   lens-client/src/test/java/org/apache/lens/client/TestLensClient.java 5cf7417eef469e9f1bfbd35fc0ecfbbe64733c8a 
>   lens-server/src/main/java/org/apache/lens/server/BaseLensService.java 9f8ee7251789cd943e7c6a1091feb44684f248c8 
>   lens-server/src/main/java/org/apache/lens/server/LensServices.java 46bae242bb627b1b542dd37c05baa2b788bd78ed 
>   lens-server/src/main/java/org/apache/lens/server/metastore/MetastoreResource.java 6f10d58ee856e264dcbc3dcba455138f9cb8110b 
>   lens-server/src/test/java/org/apache/lens/server/TestServerMode.java fce6e5f2c2d09a4d76fdcb752107b98937791275 
>   src/site/apt/user/olap-query-conf.apt 8533ca7b14c12ca8a596f89af518000238e937cd 
> 
> Diff: https://reviews.apache.org/r/47121/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Rajat Khandelwal
> 
>


Re: Review Request 47121: LENS-1010: Make LensClient implement java.lang.Closable

Posted by Amareshwari Sriramadasu <am...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/47121/#review134539
-----------------------------------------------------------


Fix it, then Ship it!





lens-cube/src/main/resources/olap-query-conf.xml (line 235)
<https://reviews.apache.org/r/47121/#comment199407>

    fix typo - falg to flag


- Amareshwari Sriramadasu


On May 24, 2016, 7:39 a.m., Rajat Khandelwal wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47121/
> -----------------------------------------------------------
> 
> (Updated May 24, 2016, 7:39 a.m.)
> 
> 
> Review request for lens.
> 
> 
> Bugs: LENS-1010
>     https://issues.apache.org/jira/browse/LENS-1010
> 
> 
> Repository: lens
> 
> 
> Description
> -------
> 
> This would allow users to use lens client in try-with-resources clause without worrying about closing manually.
> 
> https://docs.oracle.com/javase/tutorial/essential/exceptions/tryResourceClose.html
> 
> 
> Diffs
> -----
> 
>   lens-client/src/main/java/org/apache/lens/client/LensClient.java 480b6f8e5b7d98f10e5315beb41955597acbd4c3 
>   lens-client/src/main/java/org/apache/lens/client/LensConnection.java 49518662eb17c1fd24deb26ddd46a36c8826bc5b 
>   lens-client/src/test/java/org/apache/lens/client/TestLensClient.java 5cf7417eef469e9f1bfbd35fc0ecfbbe64733c8a 
>   lens-cube/src/main/resources/olap-query-conf.xml 6e9933e6b2260ee28af0030d02333d13b97163cc 
>   lens-server/src/main/java/org/apache/lens/server/BaseLensService.java 76c8d166b7ac856e6a4a4956e8dcb03584eb407b 
>   lens-server/src/main/java/org/apache/lens/server/LensServices.java 46bae242bb627b1b542dd37c05baa2b788bd78ed 
>   lens-server/src/main/java/org/apache/lens/server/metastore/MetastoreResource.java 6f10d58ee856e264dcbc3dcba455138f9cb8110b 
>   lens-server/src/test/java/org/apache/lens/server/TestServerMode.java fce6e5f2c2d09a4d76fdcb752107b98937791275 
>   src/site/apt/user/olap-query-conf.apt 8533ca7b14c12ca8a596f89af518000238e937cd 
> 
> Diff: https://reviews.apache.org/r/47121/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Rajat Khandelwal
> 
>


Re: Review Request 47121: LENS-1010: Make LensClient implement java.lang.Closable

Posted by Rajat Khandelwal <ra...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/47121/
-----------------------------------------------------------

(Updated May 24, 2016, 1:57 p.m.)


Review request for lens.


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


Repository: lens


Description
-------

This would allow users to use lens client in try-with-resources clause without worrying about closing manually.

https://docs.oracle.com/javase/tutorial/essential/exceptions/tryResourceClose.html


Diffs (updated)
-----

  lens-client/src/main/java/org/apache/lens/client/LensClient.java 480b6f8e5b7d98f10e5315beb41955597acbd4c3 
  lens-client/src/main/java/org/apache/lens/client/LensConnection.java 49518662eb17c1fd24deb26ddd46a36c8826bc5b 
  lens-client/src/test/java/org/apache/lens/client/TestLensClient.java 5cf7417eef469e9f1bfbd35fc0ecfbbe64733c8a 
  lens-cube/src/main/resources/olap-query-conf.xml 6e9933e6b2260ee28af0030d02333d13b97163cc 
  lens-server/src/main/java/org/apache/lens/server/BaseLensService.java 76c8d166b7ac856e6a4a4956e8dcb03584eb407b 
  lens-server/src/main/java/org/apache/lens/server/LensServices.java 46bae242bb627b1b542dd37c05baa2b788bd78ed 
  lens-server/src/main/java/org/apache/lens/server/metastore/MetastoreResource.java 6f10d58ee856e264dcbc3dcba455138f9cb8110b 
  lens-server/src/test/java/org/apache/lens/server/TestServerMode.java fce6e5f2c2d09a4d76fdcb752107b98937791275 
  src/site/apt/user/olap-query-conf.apt 8533ca7b14c12ca8a596f89af518000238e937cd 

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


Testing
-------


Thanks,

Rajat Khandelwal


Re: Review Request 47121: LENS-1010: Make LensClient implement java.lang.Closable

Posted by Rajat Khandelwal <ra...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/47121/
-----------------------------------------------------------

(Updated May 24, 2016, 1:09 p.m.)


Review request for lens.


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


Repository: lens


Description
-------

This would allow users to use lens client in try-with-resources clause without worrying about closing manually.

https://docs.oracle.com/javase/tutorial/essential/exceptions/tryResourceClose.html


Diffs (updated)
-----

  lens-client/src/main/java/org/apache/lens/client/LensClient.java 480b6f8e5b7d98f10e5315beb41955597acbd4c3 
  lens-client/src/main/java/org/apache/lens/client/LensConnection.java 49518662eb17c1fd24deb26ddd46a36c8826bc5b 
  lens-client/src/test/java/org/apache/lens/client/TestLensClient.java 5cf7417eef469e9f1bfbd35fc0ecfbbe64733c8a 
  lens-cube/src/main/resources/olap-query-conf.xml 6e9933e6b2260ee28af0030d02333d13b97163cc 
  lens-server/src/main/java/org/apache/lens/server/BaseLensService.java 76c8d166b7ac856e6a4a4956e8dcb03584eb407b 
  lens-server/src/main/java/org/apache/lens/server/LensServices.java 46bae242bb627b1b542dd37c05baa2b788bd78ed 
  lens-server/src/main/java/org/apache/lens/server/metastore/MetastoreResource.java 6f10d58ee856e264dcbc3dcba455138f9cb8110b 
  lens-server/src/test/java/org/apache/lens/server/TestServerMode.java fce6e5f2c2d09a4d76fdcb752107b98937791275 
  src/site/apt/user/olap-query-conf.apt 8533ca7b14c12ca8a596f89af518000238e937cd 

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


Testing
-------


Thanks,

Rajat Khandelwal


Re: Review Request 47121: LENS-1010: Make LensClient implement java.lang.Closable

Posted by Amareshwari Sriramadasu <am...@apache.org>.

> On May 23, 2016, 12:40 p.m., Puneet Gupta wrote:
> > lens-server/src/main/java/org/apache/lens/server/LensServices.java, line 144
> > <https://reviews.apache.org/r/47121/diff/8/?file=1390528#file1390528line144>
> >
> >     This is not required after Lens-957. GenericExceptionMapper should take care of this.
> >     
> >     Should we remove all usages form form MetastoreResource.java ?

This is a different processing done in MetastoreResource to get the error to be returned in APIResult. LENS-957 is not solving this, as no exception is thrown from MetastoreResource apis yet. So, we might have to keep this until MetastoreResource api is fixed.


- Amareshwari


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


On May 21, 2016, 8:33 a.m., Rajat Khandelwal wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47121/
> -----------------------------------------------------------
> 
> (Updated May 21, 2016, 8:33 a.m.)
> 
> 
> Review request for lens.
> 
> 
> Bugs: LENS-1010
>     https://issues.apache.org/jira/browse/LENS-1010
> 
> 
> Repository: lens
> 
> 
> Description
> -------
> 
> This would allow users to use lens client in try-with-resources clause without worrying about closing manually.
> 
> https://docs.oracle.com/javase/tutorial/essential/exceptions/tryResourceClose.html
> 
> 
> Diffs
> -----
> 
>   lens-client/src/main/java/org/apache/lens/client/LensClient.java 480b6f8e5b7d98f10e5315beb41955597acbd4c3 
>   lens-client/src/main/java/org/apache/lens/client/LensConnection.java 49518662eb17c1fd24deb26ddd46a36c8826bc5b 
>   lens-client/src/test/java/org/apache/lens/client/TestLensClient.java 5cf7417eef469e9f1bfbd35fc0ecfbbe64733c8a 
>   lens-server/src/main/java/org/apache/lens/server/BaseLensService.java 9f8ee7251789cd943e7c6a1091feb44684f248c8 
>   lens-server/src/main/java/org/apache/lens/server/LensServices.java 46bae242bb627b1b542dd37c05baa2b788bd78ed 
>   lens-server/src/main/java/org/apache/lens/server/metastore/MetastoreResource.java 6f10d58ee856e264dcbc3dcba455138f9cb8110b 
>   lens-server/src/test/java/org/apache/lens/server/TestServerMode.java fce6e5f2c2d09a4d76fdcb752107b98937791275 
>   src/site/apt/user/olap-query-conf.apt 8533ca7b14c12ca8a596f89af518000238e937cd 
> 
> Diff: https://reviews.apache.org/r/47121/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Rajat Khandelwal
> 
>


Re: Review Request 47121: LENS-1010: Make LensClient implement java.lang.Closable

Posted by Puneet Gupta <pu...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/47121/#review134340
-----------------------------------------------------------




lens-server/src/main/java/org/apache/lens/server/LensServices.java (line 144)
<https://reviews.apache.org/r/47121/#comment199141>

    This is not required after Lens-957. GenericExceptionMapper should take care of this.
    
    Should we remove all usages form form MetastoreResource.java ?


- Puneet Gupta


On May 21, 2016, 8:33 a.m., Rajat Khandelwal wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47121/
> -----------------------------------------------------------
> 
> (Updated May 21, 2016, 8:33 a.m.)
> 
> 
> Review request for lens.
> 
> 
> Bugs: LENS-1010
>     https://issues.apache.org/jira/browse/LENS-1010
> 
> 
> Repository: lens
> 
> 
> Description
> -------
> 
> This would allow users to use lens client in try-with-resources clause without worrying about closing manually.
> 
> https://docs.oracle.com/javase/tutorial/essential/exceptions/tryResourceClose.html
> 
> 
> Diffs
> -----
> 
>   lens-client/src/main/java/org/apache/lens/client/LensClient.java 480b6f8e5b7d98f10e5315beb41955597acbd4c3 
>   lens-client/src/main/java/org/apache/lens/client/LensConnection.java 49518662eb17c1fd24deb26ddd46a36c8826bc5b 
>   lens-client/src/test/java/org/apache/lens/client/TestLensClient.java 5cf7417eef469e9f1bfbd35fc0ecfbbe64733c8a 
>   lens-server/src/main/java/org/apache/lens/server/BaseLensService.java 9f8ee7251789cd943e7c6a1091feb44684f248c8 
>   lens-server/src/main/java/org/apache/lens/server/LensServices.java 46bae242bb627b1b542dd37c05baa2b788bd78ed 
>   lens-server/src/main/java/org/apache/lens/server/metastore/MetastoreResource.java 6f10d58ee856e264dcbc3dcba455138f9cb8110b 
>   lens-server/src/test/java/org/apache/lens/server/TestServerMode.java fce6e5f2c2d09a4d76fdcb752107b98937791275 
>   src/site/apt/user/olap-query-conf.apt 8533ca7b14c12ca8a596f89af518000238e937cd 
> 
> Diff: https://reviews.apache.org/r/47121/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Rajat Khandelwal
> 
>


Re: Review Request 47121: LENS-1010: Make LensClient implement java.lang.Closable

Posted by Puneet Gupta <pu...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/47121/#review134521
-----------------------------------------------------------


Ship it!




Ship It!

- Puneet Gupta


On May 21, 2016, 8:33 a.m., Rajat Khandelwal wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47121/
> -----------------------------------------------------------
> 
> (Updated May 21, 2016, 8:33 a.m.)
> 
> 
> Review request for lens.
> 
> 
> Bugs: LENS-1010
>     https://issues.apache.org/jira/browse/LENS-1010
> 
> 
> Repository: lens
> 
> 
> Description
> -------
> 
> This would allow users to use lens client in try-with-resources clause without worrying about closing manually.
> 
> https://docs.oracle.com/javase/tutorial/essential/exceptions/tryResourceClose.html
> 
> 
> Diffs
> -----
> 
>   lens-client/src/main/java/org/apache/lens/client/LensClient.java 480b6f8e5b7d98f10e5315beb41955597acbd4c3 
>   lens-client/src/main/java/org/apache/lens/client/LensConnection.java 49518662eb17c1fd24deb26ddd46a36c8826bc5b 
>   lens-client/src/test/java/org/apache/lens/client/TestLensClient.java 5cf7417eef469e9f1bfbd35fc0ecfbbe64733c8a 
>   lens-server/src/main/java/org/apache/lens/server/BaseLensService.java 9f8ee7251789cd943e7c6a1091feb44684f248c8 
>   lens-server/src/main/java/org/apache/lens/server/LensServices.java 46bae242bb627b1b542dd37c05baa2b788bd78ed 
>   lens-server/src/main/java/org/apache/lens/server/metastore/MetastoreResource.java 6f10d58ee856e264dcbc3dcba455138f9cb8110b 
>   lens-server/src/test/java/org/apache/lens/server/TestServerMode.java fce6e5f2c2d09a4d76fdcb752107b98937791275 
>   src/site/apt/user/olap-query-conf.apt 8533ca7b14c12ca8a596f89af518000238e937cd 
> 
> Diff: https://reviews.apache.org/r/47121/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Rajat Khandelwal
> 
>


Re: Review Request 47121: LENS-1010: Make LensClient implement java.lang.Closable

Posted by Rajat Khandelwal <ra...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/47121/
-----------------------------------------------------------

(Updated May 21, 2016, 2:03 p.m.)


Review request for lens.


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


Repository: lens


Description
-------

This would allow users to use lens client in try-with-resources clause without worrying about closing manually.

https://docs.oracle.com/javase/tutorial/essential/exceptions/tryResourceClose.html


Diffs (updated)
-----

  lens-client/src/main/java/org/apache/lens/client/LensClient.java 480b6f8e5b7d98f10e5315beb41955597acbd4c3 
  lens-client/src/main/java/org/apache/lens/client/LensConnection.java 49518662eb17c1fd24deb26ddd46a36c8826bc5b 
  lens-client/src/test/java/org/apache/lens/client/TestLensClient.java 5cf7417eef469e9f1bfbd35fc0ecfbbe64733c8a 
  lens-server/src/main/java/org/apache/lens/server/BaseLensService.java 9f8ee7251789cd943e7c6a1091feb44684f248c8 
  lens-server/src/main/java/org/apache/lens/server/LensServices.java 46bae242bb627b1b542dd37c05baa2b788bd78ed 
  lens-server/src/main/java/org/apache/lens/server/metastore/MetastoreResource.java 6f10d58ee856e264dcbc3dcba455138f9cb8110b 
  lens-server/src/test/java/org/apache/lens/server/TestServerMode.java fce6e5f2c2d09a4d76fdcb752107b98937791275 
  src/site/apt/user/olap-query-conf.apt 8533ca7b14c12ca8a596f89af518000238e937cd 

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


Testing
-------


Thanks,

Rajat Khandelwal


Re: Review Request 47121: LENS-1010: Make LensClient implement java.lang.Closable

Posted by Rajat Khandelwal <ra...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/47121/
-----------------------------------------------------------

(Updated May 18, 2016, 4:37 p.m.)


Review request for lens.


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


Repository: lens


Description
-------

This would allow users to use lens client in try-with-resources clause without worrying about closing manually.

https://docs.oracle.com/javase/tutorial/essential/exceptions/tryResourceClose.html


Diffs (updated)
-----

  lens-client/src/main/java/org/apache/lens/client/LensClient.java 480b6f8e5b7d98f10e5315beb41955597acbd4c3 
  lens-client/src/main/java/org/apache/lens/client/LensConnection.java 49518662eb17c1fd24deb26ddd46a36c8826bc5b 
  lens-client/src/test/java/org/apache/lens/client/TestLensClient.java 5cf7417eef469e9f1bfbd35fc0ecfbbe64733c8a 
  lens-server/src/main/java/org/apache/lens/server/BaseLensService.java 9f8ee7251789cd943e7c6a1091feb44684f248c8 
  lens-server/src/main/java/org/apache/lens/server/LensServices.java 46bae242bb627b1b542dd37c05baa2b788bd78ed 
  lens-server/src/main/java/org/apache/lens/server/metastore/MetastoreResource.java 6f10d58ee856e264dcbc3dcba455138f9cb8110b 

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


Testing
-------


Thanks,

Rajat Khandelwal


Re: Review Request 47121: LENS-1010: Make LensClient implement java.lang.Closable

Posted by Rajat Khandelwal <ra...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/47121/
-----------------------------------------------------------

(Updated May 18, 2016, 3:22 p.m.)


Review request for lens.


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


Repository: lens


Description
-------

This would allow users to use lens client in try-with-resources clause without worrying about closing manually.

https://docs.oracle.com/javase/tutorial/essential/exceptions/tryResourceClose.html


Diffs (updated)
-----

  lens-client/src/main/java/org/apache/lens/client/LensClient.java 43da691aff52bc108f99033fac2e8e068ae1c216 
  lens-client/src/main/java/org/apache/lens/client/LensConnection.java 49518662eb17c1fd24deb26ddd46a36c8826bc5b 
  lens-client/src/test/java/org/apache/lens/client/TestLensClient.java 5cf7417eef469e9f1bfbd35fc0ecfbbe64733c8a 
  lens-cube/src/main/java/org/apache/lens/cube/metadata/CubeMetastoreClient.java c8190bcb7cc12926bd66a70a2490c5a7684ccc6b 
  lens-server/src/main/java/org/apache/lens/server/BaseLensService.java 9f8ee7251789cd943e7c6a1091feb44684f248c8 
  lens-server/src/main/java/org/apache/lens/server/LensServices.java 46bae242bb627b1b542dd37c05baa2b788bd78ed 
  lens-server/src/main/java/org/apache/lens/server/metastore/MetastoreResource.java 7395c832b8cbe6ccde811a8b76fc2e7f97d18c4e 

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


Testing
-------


Thanks,

Rajat Khandelwal


Re: Review Request 47121: LENS-1010: Make LensClient implement java.lang.Closable

Posted by Rajat Khandelwal <ra...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/47121/
-----------------------------------------------------------

(Updated May 13, 2016, 2:45 p.m.)


Review request for lens.


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


Repository: lens


Description
-------

This would allow users to use lens client in try-with-resources clause without worrying about closing manually.

https://docs.oracle.com/javase/tutorial/essential/exceptions/tryResourceClose.html


Diffs (updated)
-----

  lens-client/src/main/java/org/apache/lens/client/LensClient.java 43da691aff52bc108f99033fac2e8e068ae1c216 
  lens-client/src/main/java/org/apache/lens/client/LensConnection.java 49518662eb17c1fd24deb26ddd46a36c8826bc5b 
  lens-client/src/test/java/org/apache/lens/client/TestLensClient.java 5cf7417eef469e9f1bfbd35fc0ecfbbe64733c8a 
  lens-server/src/main/java/org/apache/lens/server/BaseLensService.java 9f8ee7251789cd943e7c6a1091feb44684f248c8 
  lens-server/src/main/java/org/apache/lens/server/LensServices.java 46bae242bb627b1b542dd37c05baa2b788bd78ed 
  lens-server/src/main/java/org/apache/lens/server/metastore/MetastoreResource.java 7395c832b8cbe6ccde811a8b76fc2e7f97d18c4e 

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


Testing
-------


Thanks,

Rajat Khandelwal


Re: Review Request 47121: LENS-1010: Make LensClient implement java.lang.Closable

Posted by Puneet Gupta <pu...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/47121/#review132850
-----------------------------------------------------------


Fix it, then Ship it!





lens-client/src/main/java/org/apache/lens/client/LensConnection.java (line 224)
<https://reviews.apache.org/r/47121/#comment197125>

    Better to remove these retries. The cause of this failure Lens-1041 is being fixed.


- Puneet Gupta


On May 12, 2016, 7:28 a.m., Rajat Khandelwal wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47121/
> -----------------------------------------------------------
> 
> (Updated May 12, 2016, 7:28 a.m.)
> 
> 
> Review request for lens.
> 
> 
> Bugs: LENS-1010
>     https://issues.apache.org/jira/browse/LENS-1010
> 
> 
> Repository: lens
> 
> 
> Description
> -------
> 
> This would allow users to use lens client in try-with-resources clause without worrying about closing manually.
> 
> https://docs.oracle.com/javase/tutorial/essential/exceptions/tryResourceClose.html
> 
> 
> Diffs
> -----
> 
>   lens-client/src/main/java/org/apache/lens/client/LensClient.java 43da691aff52bc108f99033fac2e8e068ae1c216 
>   lens-client/src/main/java/org/apache/lens/client/LensConnection.java 49518662eb17c1fd24deb26ddd46a36c8826bc5b 
>   lens-client/src/test/java/org/apache/lens/client/TestLensClient.java ff3b888038b44dedeb137853c541fb1bb68cc0ef 
>   lens-server/src/main/java/org/apache/lens/server/BaseLensService.java 9f8ee7251789cd943e7c6a1091feb44684f248c8 
>   lens-server/src/main/java/org/apache/lens/server/LensServices.java 46bae242bb627b1b542dd37c05baa2b788bd78ed 
>   lens-server/src/main/java/org/apache/lens/server/metastore/MetastoreResource.java 7395c832b8cbe6ccde811a8b76fc2e7f97d18c4e 
> 
> Diff: https://reviews.apache.org/r/47121/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Rajat Khandelwal
> 
>


Re: Review Request 47121: LENS-1010: Make LensClient implement java.lang.Closable

Posted by Rajat Khandelwal <ra...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/47121/
-----------------------------------------------------------

(Updated May 12, 2016, 12:58 p.m.)


Review request for lens.


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


Repository: lens


Description
-------

This would allow users to use lens client in try-with-resources clause without worrying about closing manually.

https://docs.oracle.com/javase/tutorial/essential/exceptions/tryResourceClose.html


Diffs (updated)
-----

  lens-client/src/main/java/org/apache/lens/client/LensClient.java 43da691aff52bc108f99033fac2e8e068ae1c216 
  lens-client/src/main/java/org/apache/lens/client/LensConnection.java 49518662eb17c1fd24deb26ddd46a36c8826bc5b 
  lens-client/src/test/java/org/apache/lens/client/TestLensClient.java ff3b888038b44dedeb137853c541fb1bb68cc0ef 
  lens-server/src/main/java/org/apache/lens/server/BaseLensService.java 9f8ee7251789cd943e7c6a1091feb44684f248c8 
  lens-server/src/main/java/org/apache/lens/server/LensServices.java 46bae242bb627b1b542dd37c05baa2b788bd78ed 
  lens-server/src/main/java/org/apache/lens/server/metastore/MetastoreResource.java 7395c832b8cbe6ccde811a8b76fc2e7f97d18c4e 

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


Testing
-------


Thanks,

Rajat Khandelwal


Re: Review Request 47121: LENS-1010: Make LensClient implement java.lang.Closable

Posted by Rajat Khandelwal <ra...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/47121/
-----------------------------------------------------------

(Updated May 10, 2016, 6:16 p.m.)


Review request for lens.


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


Repository: lens


Description
-------

This would allow users to use lens client in try-with-resources clause without worrying about closing manually.

https://docs.oracle.com/javase/tutorial/essential/exceptions/tryResourceClose.html


Diffs (updated)
-----

  lens-client/src/main/java/org/apache/lens/client/LensClient.java 43da691aff52bc108f99033fac2e8e068ae1c216 
  lens-client/src/main/java/org/apache/lens/client/LensConnection.java 49518662eb17c1fd24deb26ddd46a36c8826bc5b 
  lens-client/src/test/java/org/apache/lens/client/TestLensClient.java ff3b888038b44dedeb137853c541fb1bb68cc0ef 
  lens-server/src/main/java/org/apache/lens/server/BaseLensService.java 9f8ee7251789cd943e7c6a1091feb44684f248c8 
  lens-server/src/main/java/org/apache/lens/server/LensServices.java 46bae242bb627b1b542dd37c05baa2b788bd78ed 
  lens-server/src/main/java/org/apache/lens/server/metastore/MetastoreResource.java 7395c832b8cbe6ccde811a8b76fc2e7f97d18c4e 

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


Testing
-------


Thanks,

Rajat Khandelwal


Re: Review Request 47121: LENS-1010: Make LensClient implement java.lang.Closable

Posted by Rajat Khandelwal <ra...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/47121/
-----------------------------------------------------------

(Updated May 9, 2016, 8:48 p.m.)


Review request for lens.


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


Repository: lens


Description
-------

This would allow users to use lens client in try-with-resources clause without worrying about closing manually.

https://docs.oracle.com/javase/tutorial/essential/exceptions/tryResourceClose.html


Diffs (updated)
-----

  lens-client/src/main/java/org/apache/lens/client/LensClient.java 43da691aff52bc108f99033fac2e8e068ae1c216 
  lens-client/src/main/java/org/apache/lens/client/LensConnection.java 49518662eb17c1fd24deb26ddd46a36c8826bc5b 

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


Testing
-------


Thanks,

Rajat Khandelwal