> 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 > >
