You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@lens.apache.org by Puneet Gupta <pu...@gmail.com> on 2016/04/21 13:42:44 UTC
Review Request 46485: Lens-1008 Support simple iterator over
HTTPResultSet for lens clients
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/46485/
-----------------------------------------------------------
Review request for lens.
Bugs: LENS-1008
https://issues.apache.org/jira/browse/LENS-1008
Repository: lens
Description
-------
The result iterator can have the following methods
public interface ResultSet
{ public boolean hasNext(); public String[] next(); public String[] getColumnNames(); }
This can be used by clients to programmatically consume the result with ease.
Have supported Zipped CSV abd CSV for now with configurable delimiter, encoding and isHeaderPresent.
It can be extend to other formats in future
Users can access this feature via easily via (Another varaiation avialble with more options)
LensClient.getCsvResultSet(QueryHandle)
LensClient.getZippedCsvResultSet(QueryHandle)
Diffs
-----
lens-client/pom.xml 4fd01fb
lens-client/src/main/java/org/apache/lens/client/LensClient.java 9626820
lens-client/src/main/java/org/apache/lens/client/exceptions/LensClientIOException.java PRE-CREATION
lens-client/src/main/java/org/apache/lens/client/resultset/AbstractResultSet.java PRE-CREATION
lens-client/src/main/java/org/apache/lens/client/resultset/CsvResultSet.java PRE-CREATION
lens-client/src/main/java/org/apache/lens/client/resultset/CsvResultSetReader.java PRE-CREATION
lens-client/src/main/java/org/apache/lens/client/resultset/ResultSet.java PRE-CREATION
lens-client/src/main/java/org/apache/lens/client/resultset/ResultSetReader.java PRE-CREATION
lens-client/src/main/java/org/apache/lens/client/resultset/ZippedCsvResultSet.java PRE-CREATION
lens-client/src/main/java/org/apache/lens/client/resultset/ZippedCsvResultSetReader.java PRE-CREATION
lens-client/src/main/java/org/apache/lens/client/resultset/ZippedResultSetReader.java PRE-CREATION
lens-client/src/test/java/org/apache/lens/client/TestLensClient.java 7a00f65
lens-client/src/test/resources/dim2-part/data.data PRE-CREATION
lens-client/src/test/resources/dim_table.xml PRE-CREATION
lens-client/src/test/resources/fact1.xml PRE-CREATION
lens-client/src/test/resources/lens-client-site.xml b356e5e
lens-client/src/test/resources/local-storage.xml PRE-CREATION
lens-client/src/test/resources/sample-cube.xml PRE-CREATION
lens-client/src/test/resources/test-detail.xml PRE-CREATION
lens-client/src/test/resources/test-dimension.xml PRE-CREATION
Diff: https://reviews.apache.org/r/46485/diff/
Testing
-------
-------------------------------------------------------
T E S T S
-------------------------------------------------------
Listening for transport dt_socket at address: 8000
Running org.apache.lens.client.TestLensClient
Tests run: 7, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 145.247 sec - in org.apache.lens.client.TestLensClient
Results :
Tests run: 7, Failures: 0, Errors: 0, Skipped: 0
Thanks,
Puneet Gupta
Re: Review Request 46485: Lens-1008 Support simple iterator over
HTTPResultSet for lens clients
Posted by Amareshwari Sriramadasu <am...@apache.org>.
> On April 22, 2016, 5:12 a.m., Amareshwari Sriramadasu wrote:
> > lens-client/src/test/java/org/apache/lens/client/TestLensClient.java, line 194
> > <https://reviews.apache.org/r/46485/diff/1/?file=1354886#file1354886line194>
> >
> > Can we run with CSVserde you are using opencsv to read result?
>
> Puneet Gupta wrote:
> Was facing some issue in MR job with CSVserde. Switched to LazySimpleSerDe which is being used in all other test cases as well. Is this OK?
Ok. Should be fine. Regression tests should test with CSVSerde.
> On April 22, 2016, 5:12 a.m., Amareshwari Sriramadasu wrote:
> > lens-client/src/test/java/org/apache/lens/client/TestLensClient.java, lines 214-216
> > <https://reviews.apache.org/r/46485/diff/1/?file=1354886#file1354886line214>
> >
> > Instead of separate getResultset methods can we have single method which calls appropriate result set looking at file extension?
>
> Puneet Gupta wrote:
> Will add two methods .
>
> 1. ResultSet getCsvHttpResultSet(QueryHandle q, Charset encoding, boolean isHeaderPresent, char delimiter,
> boolean isResultZipped)
> 2. ResultSet getCsvHttpResultSet(QueryHandle q).
>
> The second method will delegate to the first one by guessing values for encoding, isHeaderPresent and delimiter based on session parameters .
Looks fine.
- Amareshwari
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/46485/#review130033
-----------------------------------------------------------
On April 21, 2016, 11:42 a.m., Puneet Gupta wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46485/
> -----------------------------------------------------------
>
> (Updated April 21, 2016, 11:42 a.m.)
>
>
> Review request for lens.
>
>
> Bugs: LENS-1008
> https://issues.apache.org/jira/browse/LENS-1008
>
>
> Repository: lens
>
>
> Description
> -------
>
> The result iterator can have the following methods
> public interface ResultSet
> { public boolean hasNext(); public String[] next(); public String[] getColumnNames(); }
> This can be used by clients to programmatically consume the result with ease.
>
>
> Have supported Zipped CSV abd CSV for now with configurable delimiter, encoding and isHeaderPresent.
> It can be extend to other formats in future
>
> Users can access this feature via easily via (Another varaiation avialble with more options)
> LensClient.getCsvResultSet(QueryHandle)
> LensClient.getZippedCsvResultSet(QueryHandle)
>
>
> Diffs
> -----
>
> lens-client/pom.xml 4fd01fb
> lens-client/src/main/java/org/apache/lens/client/LensClient.java 9626820
> lens-client/src/main/java/org/apache/lens/client/exceptions/LensClientIOException.java PRE-CREATION
> lens-client/src/main/java/org/apache/lens/client/resultset/AbstractResultSet.java PRE-CREATION
> lens-client/src/main/java/org/apache/lens/client/resultset/CsvResultSet.java PRE-CREATION
> lens-client/src/main/java/org/apache/lens/client/resultset/CsvResultSetReader.java PRE-CREATION
> lens-client/src/main/java/org/apache/lens/client/resultset/ResultSet.java PRE-CREATION
> lens-client/src/main/java/org/apache/lens/client/resultset/ResultSetReader.java PRE-CREATION
> lens-client/src/main/java/org/apache/lens/client/resultset/ZippedCsvResultSet.java PRE-CREATION
> lens-client/src/main/java/org/apache/lens/client/resultset/ZippedCsvResultSetReader.java PRE-CREATION
> lens-client/src/main/java/org/apache/lens/client/resultset/ZippedResultSetReader.java PRE-CREATION
> lens-client/src/test/java/org/apache/lens/client/TestLensClient.java 7a00f65
> lens-client/src/test/resources/dim2-part/data.data PRE-CREATION
> lens-client/src/test/resources/dim_table.xml PRE-CREATION
> lens-client/src/test/resources/fact1.xml PRE-CREATION
> lens-client/src/test/resources/lens-client-site.xml b356e5e
> lens-client/src/test/resources/local-storage.xml PRE-CREATION
> lens-client/src/test/resources/sample-cube.xml PRE-CREATION
> lens-client/src/test/resources/test-detail.xml PRE-CREATION
> lens-client/src/test/resources/test-dimension.xml PRE-CREATION
>
> Diff: https://reviews.apache.org/r/46485/diff/
>
>
> Testing
> -------
>
> -------------------------------------------------------
> T E S T S
> -------------------------------------------------------
> Listening for transport dt_socket at address: 8000
> Running org.apache.lens.client.TestLensClient
> Tests run: 7, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 145.247 sec - in org.apache.lens.client.TestLensClient
>
> Results :
>
> Tests run: 7, Failures: 0, Errors: 0, Skipped: 0
>
>
> Thanks,
>
> Puneet Gupta
>
>
Re: Review Request 46485: Lens-1008 Support simple iterator over
HTTPResultSet for lens clients
Posted by Puneet Gupta <pu...@gmail.com>.
> On April 22, 2016, 5:12 a.m., Amareshwari Sriramadasu wrote:
> > lens-client/src/test/java/org/apache/lens/client/TestLensClient.java, lines 214-216
> > <https://reviews.apache.org/r/46485/diff/1/?file=1354886#file1354886line214>
> >
> > Instead of separate getResultset methods can we have single method which calls appropriate result set looking at file extension?
Will add two methods .
1. ResultSet getCsvHttpResultSet(QueryHandle q, Charset encoding, boolean isHeaderPresent, char delimiter,
boolean isResultZipped)
2. ResultSet getCsvHttpResultSet(QueryHandle q).
The second method will delegate to the first one by guessing values for encoding, isHeaderPresent and delimiter based on session parameters .
> On April 22, 2016, 5:12 a.m., Amareshwari Sriramadasu wrote:
> > lens-client/src/test/java/org/apache/lens/client/TestLensClient.java, line 48
> > <https://reviews.apache.org/r/46485/diff/1/?file=1354886#file1354886line48>
> >
> > Can you remove if group is not required?
commented by mistake. Uncommented.
> On April 22, 2016, 5:12 a.m., Amareshwari Sriramadasu wrote:
> > lens-client/src/test/java/org/apache/lens/client/TestLensClient.java, line 194
> > <https://reviews.apache.org/r/46485/diff/1/?file=1354886#file1354886line194>
> >
> > Can we run with CSVserde you are using opencsv to read result?
Was facing some issue in MR job with CSVserde. Switched to LazySimpleSerDe which is being used in all other test cases as well. Is this OK?
- Puneet
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/46485/#review130033
-----------------------------------------------------------
On April 21, 2016, 11:42 a.m., Puneet Gupta wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46485/
> -----------------------------------------------------------
>
> (Updated April 21, 2016, 11:42 a.m.)
>
>
> Review request for lens.
>
>
> Bugs: LENS-1008
> https://issues.apache.org/jira/browse/LENS-1008
>
>
> Repository: lens
>
>
> Description
> -------
>
> The result iterator can have the following methods
> public interface ResultSet
> { public boolean hasNext(); public String[] next(); public String[] getColumnNames(); }
> This can be used by clients to programmatically consume the result with ease.
>
>
> Have supported Zipped CSV abd CSV for now with configurable delimiter, encoding and isHeaderPresent.
> It can be extend to other formats in future
>
> Users can access this feature via easily via (Another varaiation avialble with more options)
> LensClient.getCsvResultSet(QueryHandle)
> LensClient.getZippedCsvResultSet(QueryHandle)
>
>
> Diffs
> -----
>
> lens-client/pom.xml 4fd01fb
> lens-client/src/main/java/org/apache/lens/client/LensClient.java 9626820
> lens-client/src/main/java/org/apache/lens/client/exceptions/LensClientIOException.java PRE-CREATION
> lens-client/src/main/java/org/apache/lens/client/resultset/AbstractResultSet.java PRE-CREATION
> lens-client/src/main/java/org/apache/lens/client/resultset/CsvResultSet.java PRE-CREATION
> lens-client/src/main/java/org/apache/lens/client/resultset/CsvResultSetReader.java PRE-CREATION
> lens-client/src/main/java/org/apache/lens/client/resultset/ResultSet.java PRE-CREATION
> lens-client/src/main/java/org/apache/lens/client/resultset/ResultSetReader.java PRE-CREATION
> lens-client/src/main/java/org/apache/lens/client/resultset/ZippedCsvResultSet.java PRE-CREATION
> lens-client/src/main/java/org/apache/lens/client/resultset/ZippedCsvResultSetReader.java PRE-CREATION
> lens-client/src/main/java/org/apache/lens/client/resultset/ZippedResultSetReader.java PRE-CREATION
> lens-client/src/test/java/org/apache/lens/client/TestLensClient.java 7a00f65
> lens-client/src/test/resources/dim2-part/data.data PRE-CREATION
> lens-client/src/test/resources/dim_table.xml PRE-CREATION
> lens-client/src/test/resources/fact1.xml PRE-CREATION
> lens-client/src/test/resources/lens-client-site.xml b356e5e
> lens-client/src/test/resources/local-storage.xml PRE-CREATION
> lens-client/src/test/resources/sample-cube.xml PRE-CREATION
> lens-client/src/test/resources/test-detail.xml PRE-CREATION
> lens-client/src/test/resources/test-dimension.xml PRE-CREATION
>
> Diff: https://reviews.apache.org/r/46485/diff/
>
>
> Testing
> -------
>
> -------------------------------------------------------
> T E S T S
> -------------------------------------------------------
> Listening for transport dt_socket at address: 8000
> Running org.apache.lens.client.TestLensClient
> Tests run: 7, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 145.247 sec - in org.apache.lens.client.TestLensClient
>
> Results :
>
> Tests run: 7, Failures: 0, Errors: 0, Skipped: 0
>
>
> Thanks,
>
> Puneet Gupta
>
>
Re: Review Request 46485: Lens-1008 Support simple iterator over
HTTPResultSet for lens clients
Posted by Amareshwari Sriramadasu <am...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/46485/#review130033
-----------------------------------------------------------
lens-client/src/test/java/org/apache/lens/client/TestLensClient.java (line 44)
<https://reviews.apache.org/r/46485/#comment193647>
Can you remove if group is not required?
lens-client/src/test/java/org/apache/lens/client/TestLensClient.java (line 189)
<https://reviews.apache.org/r/46485/#comment193648>
Can we run with CSVserde you are using opencsv to read result?
lens-client/src/test/java/org/apache/lens/client/TestLensClient.java (lines 209 - 211)
<https://reviews.apache.org/r/46485/#comment193649>
Instead of separate getResultset methods can we have single method which calls appropriate result set looking at file extension?
- Amareshwari Sriramadasu
On April 21, 2016, 11:42 a.m., Puneet Gupta wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46485/
> -----------------------------------------------------------
>
> (Updated April 21, 2016, 11:42 a.m.)
>
>
> Review request for lens.
>
>
> Bugs: LENS-1008
> https://issues.apache.org/jira/browse/LENS-1008
>
>
> Repository: lens
>
>
> Description
> -------
>
> The result iterator can have the following methods
> public interface ResultSet
> { public boolean hasNext(); public String[] next(); public String[] getColumnNames(); }
> This can be used by clients to programmatically consume the result with ease.
>
>
> Have supported Zipped CSV abd CSV for now with configurable delimiter, encoding and isHeaderPresent.
> It can be extend to other formats in future
>
> Users can access this feature via easily via (Another varaiation avialble with more options)
> LensClient.getCsvResultSet(QueryHandle)
> LensClient.getZippedCsvResultSet(QueryHandle)
>
>
> Diffs
> -----
>
> lens-client/pom.xml 4fd01fb
> lens-client/src/main/java/org/apache/lens/client/LensClient.java 9626820
> lens-client/src/main/java/org/apache/lens/client/exceptions/LensClientIOException.java PRE-CREATION
> lens-client/src/main/java/org/apache/lens/client/resultset/AbstractResultSet.java PRE-CREATION
> lens-client/src/main/java/org/apache/lens/client/resultset/CsvResultSet.java PRE-CREATION
> lens-client/src/main/java/org/apache/lens/client/resultset/CsvResultSetReader.java PRE-CREATION
> lens-client/src/main/java/org/apache/lens/client/resultset/ResultSet.java PRE-CREATION
> lens-client/src/main/java/org/apache/lens/client/resultset/ResultSetReader.java PRE-CREATION
> lens-client/src/main/java/org/apache/lens/client/resultset/ZippedCsvResultSet.java PRE-CREATION
> lens-client/src/main/java/org/apache/lens/client/resultset/ZippedCsvResultSetReader.java PRE-CREATION
> lens-client/src/main/java/org/apache/lens/client/resultset/ZippedResultSetReader.java PRE-CREATION
> lens-client/src/test/java/org/apache/lens/client/TestLensClient.java 7a00f65
> lens-client/src/test/resources/dim2-part/data.data PRE-CREATION
> lens-client/src/test/resources/dim_table.xml PRE-CREATION
> lens-client/src/test/resources/fact1.xml PRE-CREATION
> lens-client/src/test/resources/lens-client-site.xml b356e5e
> lens-client/src/test/resources/local-storage.xml PRE-CREATION
> lens-client/src/test/resources/sample-cube.xml PRE-CREATION
> lens-client/src/test/resources/test-detail.xml PRE-CREATION
> lens-client/src/test/resources/test-dimension.xml PRE-CREATION
>
> Diff: https://reviews.apache.org/r/46485/diff/
>
>
> Testing
> -------
>
> -------------------------------------------------------
> T E S T S
> -------------------------------------------------------
> Listening for transport dt_socket at address: 8000
> Running org.apache.lens.client.TestLensClient
> Tests run: 7, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 145.247 sec - in org.apache.lens.client.TestLensClient
>
> Results :
>
> Tests run: 7, Failures: 0, Errors: 0, Skipped: 0
>
>
> Thanks,
>
> Puneet Gupta
>
>
Re: Review Request 46485: Lens-1008 Support simple iterator over
HTTPResultSet for lens clients
Posted by Puneet Gupta <pu...@gmail.com>.
> On April 22, 2016, 8:09 a.m., Rajat Khandelwal wrote:
> > lens-client/src/main/java/org/apache/lens/client/exceptions/LensClientIOException.java, line 22
> > <https://reviews.apache.org/r/46485/diff/1/?file=1354877#file1354877line22>
> >
> > should it be `extends IOException`?
Good Suggestion . Will do that .
Checked , most places IOException is being thrown as LensClientIOExecption, execpt one place where we are calling LensStatement#getHttpResultSet(org.apache.lens.api.query.LensQuery). Here a run time execption is being thrown. Will catch this in lens client and throw LensClientIOExecption to not hassle the user with handling mutiple exceptions.
> On April 22, 2016, 8:09 a.m., Rajat Khandelwal wrote:
> > lens-client/src/main/java/org/apache/lens/client/resultset/AbstractResultSet.java, line 94
> > <https://reviews.apache.org/r/46485/diff/1/?file=1354878#file1354878line94>
> >
> > Let's make the class implement `Closeable`.
> >
> > Secondly, it seems like `throws` is redundant.
- The execpetion is kept so that the subcalsses can thorw execption if required. Thought, as of now none of the subcalsses are overriding this method .
- Not implementing Closeable as this is more of an internal method (will not be available publically)
> On April 22, 2016, 8:09 a.m., Rajat Khandelwal wrote:
> > lens-client/src/main/java/org/apache/lens/client/resultset/CsvResultSetReader.java, lines 43-45
> > <https://reviews.apache.org/r/46485/diff/1/?file=1354880#file1354880line43>
> >
> > Multiple `getNext` calls will return same line unless combined with `hasNext`. This is counter intuitive while designing an iterator. Refer http://stackoverflow.com/a/29061917
Was trying to mimic jdbc result set where the hasNext() (actually its called boolean next() which makes more sense ) moves the cursor by one position.
Will change method names
boolean hasNext() = booelan next();
String[] next() = String[] getRow()/getCurrentRow();
The users should use it like JDBC ResultSet. This should remove the confusion around mutiple ways to access it.
> On April 22, 2016, 8:09 a.m., Rajat Khandelwal wrote:
> > lens-client/src/main/java/org/apache/lens/client/resultset/ResultSet.java, line 29
> > <https://reviews.apache.org/r/46485/diff/1/?file=1354881#file1354881line29>
> >
> > I think this can also implement `Iterator<String[]>` which will imply that the syntax like the following is supported:
> >
> > ```
> > ResultSet rs = ...;
> > for(String[] line: rs) {
> > ...
> > }
> > ```
The Iterator syntax which you suggested looks tempting.
Looked at java.util.Iterator . It has more methods which will not make sense (remove, forEachRemaining). More methods can be added in future too. Want to stick to booelan next() and String[] getRow() and not expose other dummy methods from iterator.
> On April 22, 2016, 8:09 a.m., Rajat Khandelwal wrote:
> > lens-client/src/main/java/org/apache/lens/client/resultset/ZippedCsvResultSet.java, line 35
> > <https://reviews.apache.org/r/46485/diff/1/?file=1354883#file1354883line35>
> >
> > Return type here can be `ZippedCsvResultSetReader` so that someone testing `ZippedCsvResultSet` can directly expect that type when calling `createResultSetReader` without having to type-cast.
As of now all test cases are coded to interface and not specific implemenatations. Also having the interface as reutn type will give subclasses an option to return a different implemenattion type in future if required. Don't see a pressing need to expose actual type here.
- Puneet
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/46485/#review130059
-----------------------------------------------------------
On April 21, 2016, 11:42 a.m., Puneet Gupta wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46485/
> -----------------------------------------------------------
>
> (Updated April 21, 2016, 11:42 a.m.)
>
>
> Review request for lens.
>
>
> Bugs: LENS-1008
> https://issues.apache.org/jira/browse/LENS-1008
>
>
> Repository: lens
>
>
> Description
> -------
>
> The result iterator can have the following methods
> public interface ResultSet
> { public boolean hasNext(); public String[] next(); public String[] getColumnNames(); }
> This can be used by clients to programmatically consume the result with ease.
>
>
> Have supported Zipped CSV abd CSV for now with configurable delimiter, encoding and isHeaderPresent.
> It can be extend to other formats in future
>
> Users can access this feature via easily via (Another varaiation avialble with more options)
> LensClient.getCsvResultSet(QueryHandle)
> LensClient.getZippedCsvResultSet(QueryHandle)
>
>
> Diffs
> -----
>
> lens-client/pom.xml 4fd01fb
> lens-client/src/main/java/org/apache/lens/client/LensClient.java 9626820
> lens-client/src/main/java/org/apache/lens/client/exceptions/LensClientIOException.java PRE-CREATION
> lens-client/src/main/java/org/apache/lens/client/resultset/AbstractResultSet.java PRE-CREATION
> lens-client/src/main/java/org/apache/lens/client/resultset/CsvResultSet.java PRE-CREATION
> lens-client/src/main/java/org/apache/lens/client/resultset/CsvResultSetReader.java PRE-CREATION
> lens-client/src/main/java/org/apache/lens/client/resultset/ResultSet.java PRE-CREATION
> lens-client/src/main/java/org/apache/lens/client/resultset/ResultSetReader.java PRE-CREATION
> lens-client/src/main/java/org/apache/lens/client/resultset/ZippedCsvResultSet.java PRE-CREATION
> lens-client/src/main/java/org/apache/lens/client/resultset/ZippedCsvResultSetReader.java PRE-CREATION
> lens-client/src/main/java/org/apache/lens/client/resultset/ZippedResultSetReader.java PRE-CREATION
> lens-client/src/test/java/org/apache/lens/client/TestLensClient.java 7a00f65
> lens-client/src/test/resources/dim2-part/data.data PRE-CREATION
> lens-client/src/test/resources/dim_table.xml PRE-CREATION
> lens-client/src/test/resources/fact1.xml PRE-CREATION
> lens-client/src/test/resources/lens-client-site.xml b356e5e
> lens-client/src/test/resources/local-storage.xml PRE-CREATION
> lens-client/src/test/resources/sample-cube.xml PRE-CREATION
> lens-client/src/test/resources/test-detail.xml PRE-CREATION
> lens-client/src/test/resources/test-dimension.xml PRE-CREATION
>
> Diff: https://reviews.apache.org/r/46485/diff/
>
>
> Testing
> -------
>
> -------------------------------------------------------
> T E S T S
> -------------------------------------------------------
> Listening for transport dt_socket at address: 8000
> Running org.apache.lens.client.TestLensClient
> Tests run: 7, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 145.247 sec - in org.apache.lens.client.TestLensClient
>
> Results :
>
> Tests run: 7, Failures: 0, Errors: 0, Skipped: 0
>
>
> Thanks,
>
> Puneet Gupta
>
>
Re: Review Request 46485: Lens-1008 Support simple iterator over
HTTPResultSet for lens clients
Posted by Puneet Gupta <pu...@gmail.com>.
> On April 22, 2016, 8:09 a.m., Rajat Khandelwal wrote:
> > lens-client/src/main/java/org/apache/lens/client/resultset/ResultSet.java, line 29
> > <https://reviews.apache.org/r/46485/diff/1/?file=1354881#file1354881line29>
> >
> > I think this can also implement `Iterator<String[]>` which will imply that the syntax like the following is supported:
> >
> > ```
> > ResultSet rs = ...;
> > for(String[] line: rs) {
> > ...
> > }
> > ```
>
> Puneet Gupta wrote:
> The Iterator syntax which you suggested looks tempting.
>
> Looked at java.util.Iterator . It has more methods which will not make sense (remove, forEachRemaining). More methods can be added in future too. Want to stick to booelan next() and String[] getRow() and not expose other dummy methods from iterator.
Execption handling in Iterator interface is also a blocker. Will have to throw a RunTimeExecption
- Puneet
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/46485/#review130059
-----------------------------------------------------------
On April 25, 2016, 11:12 a.m., Puneet Gupta wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46485/
> -----------------------------------------------------------
>
> (Updated April 25, 2016, 11:12 a.m.)
>
>
> Review request for lens.
>
>
> Bugs: LENS-1008
> https://issues.apache.org/jira/browse/LENS-1008
>
>
> Repository: lens
>
>
> Description
> -------
>
> The result iterator can have the following methods
> public interface ResultSet
> { public boolean hasNext(); public String[] next(); public String[] getColumnNames(); }
> This can be used by clients to programmatically consume the result with ease.
>
>
> Have supported Zipped CSV abd CSV for now with configurable delimiter, encoding and isHeaderPresent.
> It can be extend to other formats in future
>
> Users can access this feature via easily via (Another varaiation avialble with more options)
> LensClient.getCsvResultSet(QueryHandle)
> LensClient.getZippedCsvResultSet(QueryHandle)
>
>
> Diffs
> -----
>
> lens-client/pom.xml 4fd01fb
> lens-client/src/main/java/org/apache/lens/client/LensClient.java 9626820
> lens-client/src/main/java/org/apache/lens/client/LensConnection.java d67e64e
> lens-client/src/main/java/org/apache/lens/client/exceptions/LensClientIOException.java PRE-CREATION
> lens-client/src/main/java/org/apache/lens/client/resultset/AbstractResultSet.java PRE-CREATION
> lens-client/src/main/java/org/apache/lens/client/resultset/CsvResultSet.java PRE-CREATION
> lens-client/src/main/java/org/apache/lens/client/resultset/CsvResultSetReader.java PRE-CREATION
> lens-client/src/main/java/org/apache/lens/client/resultset/ResultSet.java PRE-CREATION
> lens-client/src/main/java/org/apache/lens/client/resultset/ResultSetReader.java PRE-CREATION
> lens-client/src/main/java/org/apache/lens/client/resultset/ZippedCsvResultSet.java PRE-CREATION
> lens-client/src/main/java/org/apache/lens/client/resultset/ZippedCsvResultSetReader.java PRE-CREATION
> lens-client/src/main/java/org/apache/lens/client/resultset/ZippedResultSetReader.java PRE-CREATION
> lens-client/src/test/java/org/apache/lens/client/TestLensClient.java 7a00f65
> lens-client/src/test/resources/dim2-part/data.data PRE-CREATION
> lens-client/src/test/resources/dim_table.xml PRE-CREATION
> lens-client/src/test/resources/fact1.xml PRE-CREATION
> lens-client/src/test/resources/lens-client-site.xml b356e5e
> lens-client/src/test/resources/local-storage.xml PRE-CREATION
> lens-client/src/test/resources/sample-cube.xml PRE-CREATION
> lens-client/src/test/resources/test-detail.xml PRE-CREATION
> lens-client/src/test/resources/test-dimension.xml PRE-CREATION
>
> Diff: https://reviews.apache.org/r/46485/diff/
>
>
> Testing
> -------
>
> -------------------------------------------------------
> T E S T S
> -------------------------------------------------------
> Listening for transport dt_socket at address: 8000
> Running org.apache.lens.client.TestLensClient
> Tests run: 7, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 145.247 sec - in org.apache.lens.client.TestLensClient
>
> Results :
>
> Tests run: 7, Failures: 0, Errors: 0, Skipped: 0
>
>
> Thanks,
>
> Puneet Gupta
>
>
Re: Review Request 46485: Lens-1008 Support simple iterator over
HTTPResultSet for lens clients
Posted by Rajat Khandelwal <ra...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/46485/#review130059
-----------------------------------------------------------
lens-client/src/main/java/org/apache/lens/client/exceptions/LensClientIOException.java (line 22)
<https://reviews.apache.org/r/46485/#comment193681>
should it be `extends IOException`?
lens-client/src/main/java/org/apache/lens/client/resultset/AbstractResultSet.java (line 94)
<https://reviews.apache.org/r/46485/#comment193683>
Let's make the class implement `Closeable`.
Secondly, it seems like `throws` is redundant.
lens-client/src/main/java/org/apache/lens/client/resultset/CsvResultSetReader.java (lines 43 - 45)
<https://reviews.apache.org/r/46485/#comment193688>
Multiple `getNext` calls will return same line unless combined with `hasNext`. This is counter intuitive while designing an iterator. Refer http://stackoverflow.com/a/29061917
lens-client/src/main/java/org/apache/lens/client/resultset/ResultSet.java (line 29)
<https://reviews.apache.org/r/46485/#comment193691>
I think this can also implement `Iterator<String[]>` which will imply that the syntax like the following is supported:
```
ResultSet rs = ...;
for(String[] line: rs) {
...
}
```
lens-client/src/main/java/org/apache/lens/client/resultset/ResultSetReader.java (line 24)
<https://reviews.apache.org/r/46485/#comment193685>
`implements Iterator<String[]>`?
lens-client/src/main/java/org/apache/lens/client/resultset/ZippedCsvResultSet.java (line 35)
<https://reviews.apache.org/r/46485/#comment193692>
Return type here can be `ZippedCsvResultSetReader` so that someone testing `ZippedCsvResultSet` can directly expect that type when calling `createResultSetReader` without having to type-cast.
- Rajat Khandelwal
On April 21, 2016, 5:12 p.m., Puneet Gupta wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46485/
> -----------------------------------------------------------
>
> (Updated April 21, 2016, 5:12 p.m.)
>
>
> Review request for lens.
>
>
> Bugs: LENS-1008
> https://issues.apache.org/jira/browse/LENS-1008
>
>
> Repository: lens
>
>
> Description
> -------
>
> The result iterator can have the following methods
> public interface ResultSet
> { public boolean hasNext(); public String[] next(); public String[] getColumnNames(); }
> This can be used by clients to programmatically consume the result with ease.
>
>
> Have supported Zipped CSV abd CSV for now with configurable delimiter, encoding and isHeaderPresent.
> It can be extend to other formats in future
>
> Users can access this feature via easily via (Another varaiation avialble with more options)
> LensClient.getCsvResultSet(QueryHandle)
> LensClient.getZippedCsvResultSet(QueryHandle)
>
>
> Diffs
> -----
>
> lens-client/pom.xml 4fd01fb
> lens-client/src/main/java/org/apache/lens/client/LensClient.java 9626820
> lens-client/src/main/java/org/apache/lens/client/exceptions/LensClientIOException.java PRE-CREATION
> lens-client/src/main/java/org/apache/lens/client/resultset/AbstractResultSet.java PRE-CREATION
> lens-client/src/main/java/org/apache/lens/client/resultset/CsvResultSet.java PRE-CREATION
> lens-client/src/main/java/org/apache/lens/client/resultset/CsvResultSetReader.java PRE-CREATION
> lens-client/src/main/java/org/apache/lens/client/resultset/ResultSet.java PRE-CREATION
> lens-client/src/main/java/org/apache/lens/client/resultset/ResultSetReader.java PRE-CREATION
> lens-client/src/main/java/org/apache/lens/client/resultset/ZippedCsvResultSet.java PRE-CREATION
> lens-client/src/main/java/org/apache/lens/client/resultset/ZippedCsvResultSetReader.java PRE-CREATION
> lens-client/src/main/java/org/apache/lens/client/resultset/ZippedResultSetReader.java PRE-CREATION
> lens-client/src/test/java/org/apache/lens/client/TestLensClient.java 7a00f65
> lens-client/src/test/resources/dim2-part/data.data PRE-CREATION
> lens-client/src/test/resources/dim_table.xml PRE-CREATION
> lens-client/src/test/resources/fact1.xml PRE-CREATION
> lens-client/src/test/resources/lens-client-site.xml b356e5e
> lens-client/src/test/resources/local-storage.xml PRE-CREATION
> lens-client/src/test/resources/sample-cube.xml PRE-CREATION
> lens-client/src/test/resources/test-detail.xml PRE-CREATION
> lens-client/src/test/resources/test-dimension.xml PRE-CREATION
>
> Diff: https://reviews.apache.org/r/46485/diff/
>
>
> Testing
> -------
>
> -------------------------------------------------------
> T E S T S
> -------------------------------------------------------
> Listening for transport dt_socket at address: 8000
> Running org.apache.lens.client.TestLensClient
> Tests run: 7, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 145.247 sec - in org.apache.lens.client.TestLensClient
>
> Results :
>
> Tests run: 7, Failures: 0, Errors: 0, Skipped: 0
>
>
> Thanks,
>
> Puneet Gupta
>
>
Re: Review Request 46485: Lens-1008 Support simple iterator over
HTTPResultSet for lens clients
Posted by Amareshwari Sriramadasu <am...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/46485/#review130599
-----------------------------------------------------------
Ship it!
Ship It!
- Amareshwari Sriramadasu
On April 25, 2016, 11:12 a.m., Puneet Gupta wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46485/
> -----------------------------------------------------------
>
> (Updated April 25, 2016, 11:12 a.m.)
>
>
> Review request for lens.
>
>
> Bugs: LENS-1008
> https://issues.apache.org/jira/browse/LENS-1008
>
>
> Repository: lens
>
>
> Description
> -------
>
> The result iterator can have the following methods
> public interface ResultSet
> { public boolean hasNext(); public String[] next(); public String[] getColumnNames(); }
> This can be used by clients to programmatically consume the result with ease.
>
>
> Have supported Zipped CSV abd CSV for now with configurable delimiter, encoding and isHeaderPresent.
> It can be extend to other formats in future
>
> Users can access this feature via easily via (Another varaiation avialble with more options)
> LensClient.getCsvResultSet(QueryHandle)
> LensClient.getZippedCsvResultSet(QueryHandle)
>
>
> Diffs
> -----
>
> lens-client/pom.xml 4fd01fb
> lens-client/src/main/java/org/apache/lens/client/LensClient.java 9626820
> lens-client/src/main/java/org/apache/lens/client/LensConnection.java d67e64e
> lens-client/src/main/java/org/apache/lens/client/exceptions/LensClientIOException.java PRE-CREATION
> lens-client/src/main/java/org/apache/lens/client/resultset/AbstractResultSet.java PRE-CREATION
> lens-client/src/main/java/org/apache/lens/client/resultset/CsvResultSet.java PRE-CREATION
> lens-client/src/main/java/org/apache/lens/client/resultset/CsvResultSetReader.java PRE-CREATION
> lens-client/src/main/java/org/apache/lens/client/resultset/ResultSet.java PRE-CREATION
> lens-client/src/main/java/org/apache/lens/client/resultset/ResultSetReader.java PRE-CREATION
> lens-client/src/main/java/org/apache/lens/client/resultset/ZippedCsvResultSet.java PRE-CREATION
> lens-client/src/main/java/org/apache/lens/client/resultset/ZippedCsvResultSetReader.java PRE-CREATION
> lens-client/src/main/java/org/apache/lens/client/resultset/ZippedResultSetReader.java PRE-CREATION
> lens-client/src/test/java/org/apache/lens/client/TestLensClient.java 7a00f65
> lens-client/src/test/resources/dim2-part/data.data PRE-CREATION
> lens-client/src/test/resources/dim_table.xml PRE-CREATION
> lens-client/src/test/resources/fact1.xml PRE-CREATION
> lens-client/src/test/resources/lens-client-site.xml b356e5e
> lens-client/src/test/resources/local-storage.xml PRE-CREATION
> lens-client/src/test/resources/sample-cube.xml PRE-CREATION
> lens-client/src/test/resources/test-detail.xml PRE-CREATION
> lens-client/src/test/resources/test-dimension.xml PRE-CREATION
>
> Diff: https://reviews.apache.org/r/46485/diff/
>
>
> Testing
> -------
>
> -------------------------------------------------------
> T E S T S
> -------------------------------------------------------
> Listening for transport dt_socket at address: 8000
> Running org.apache.lens.client.TestLensClient
> Tests run: 7, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 145.247 sec - in org.apache.lens.client.TestLensClient
>
> Results :
>
> Tests run: 7, Failures: 0, Errors: 0, Skipped: 0
>
>
> Thanks,
>
> Puneet Gupta
>
>
Re: Review Request 46485: Lens-1008 Support simple iterator over
HTTPResultSet for lens clients
Posted by Puneet Gupta <pu...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/46485/
-----------------------------------------------------------
(Updated April 25, 2016, 11:12 a.m.)
Review request for lens.
Changes
-------
Updated to address review comments
Bugs: LENS-1008
https://issues.apache.org/jira/browse/LENS-1008
Repository: lens
Description
-------
The result iterator can have the following methods
public interface ResultSet
{ public boolean hasNext(); public String[] next(); public String[] getColumnNames(); }
This can be used by clients to programmatically consume the result with ease.
Have supported Zipped CSV abd CSV for now with configurable delimiter, encoding and isHeaderPresent.
It can be extend to other formats in future
Users can access this feature via easily via (Another varaiation avialble with more options)
LensClient.getCsvResultSet(QueryHandle)
LensClient.getZippedCsvResultSet(QueryHandle)
Diffs (updated)
-----
lens-client/pom.xml 4fd01fb
lens-client/src/main/java/org/apache/lens/client/LensClient.java 9626820
lens-client/src/main/java/org/apache/lens/client/LensConnection.java d67e64e
lens-client/src/main/java/org/apache/lens/client/exceptions/LensClientIOException.java PRE-CREATION
lens-client/src/main/java/org/apache/lens/client/resultset/AbstractResultSet.java PRE-CREATION
lens-client/src/main/java/org/apache/lens/client/resultset/CsvResultSet.java PRE-CREATION
lens-client/src/main/java/org/apache/lens/client/resultset/CsvResultSetReader.java PRE-CREATION
lens-client/src/main/java/org/apache/lens/client/resultset/ResultSet.java PRE-CREATION
lens-client/src/main/java/org/apache/lens/client/resultset/ResultSetReader.java PRE-CREATION
lens-client/src/main/java/org/apache/lens/client/resultset/ZippedCsvResultSet.java PRE-CREATION
lens-client/src/main/java/org/apache/lens/client/resultset/ZippedCsvResultSetReader.java PRE-CREATION
lens-client/src/main/java/org/apache/lens/client/resultset/ZippedResultSetReader.java PRE-CREATION
lens-client/src/test/java/org/apache/lens/client/TestLensClient.java 7a00f65
lens-client/src/test/resources/dim2-part/data.data PRE-CREATION
lens-client/src/test/resources/dim_table.xml PRE-CREATION
lens-client/src/test/resources/fact1.xml PRE-CREATION
lens-client/src/test/resources/lens-client-site.xml b356e5e
lens-client/src/test/resources/local-storage.xml PRE-CREATION
lens-client/src/test/resources/sample-cube.xml PRE-CREATION
lens-client/src/test/resources/test-detail.xml PRE-CREATION
lens-client/src/test/resources/test-dimension.xml PRE-CREATION
Diff: https://reviews.apache.org/r/46485/diff/
Testing
-------
-------------------------------------------------------
T E S T S
-------------------------------------------------------
Listening for transport dt_socket at address: 8000
Running org.apache.lens.client.TestLensClient
Tests run: 7, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 145.247 sec - in org.apache.lens.client.TestLensClient
Results :
Tests run: 7, Failures: 0, Errors: 0, Skipped: 0
Thanks,
Puneet Gupta