----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37934/#review97907 -----------------------------------------------------------
lens-api/src/main/resources/lens-errors.conf (line 58) <https://reviews.apache.org/r/37934/#comment153977> Should user name also go in error message? Will "%s access" say admin/read/write access ? What is %s:%s ? Should that be single param %s ? lens-server-api/src/main/java/org/apache/lens/server/api/query/save/Parameter.java (line 37) <https://reviews.apache.org/r/37934/#comment153978> If this requires interaction with user/client, it should be lens-api package. lens-server-api/src/main/java/org/apache/lens/server/api/query/save/Parameter.java (line 38) <https://reviews.apache.org/r/37934/#comment153979> Please add javadoc to the class and each variable (sothat lombak doc gets generated), explaining details about the class. lens-server-api/src/main/java/org/apache/lens/server/api/query/save/ParameterCollectionType.java (line 31) <https://reviews.apache.org/r/37934/#comment153980> Same as above - If this requires interaction with user/client, it should be lens-api package. lens-server-api/src/main/java/org/apache/lens/server/api/query/save/ParameterDataType.java (line 27) <https://reviews.apache.org/r/37934/#comment153981> If this requires interaction with user/client, it should be lens-api package. lens-server-api/src/main/java/org/apache/lens/server/api/query/save/ParameterParserResponse.java (line 31) <https://reviews.apache.org/r/37934/#comment153982> If this requires interaction with user/client, it should be lens-api package. lens-server-api/src/main/java/org/apache/lens/server/api/query/save/SavedQuery.java (line 40) <https://reviews.apache.org/r/37934/#comment153983> If this requires interaction with user/client, it should be lens-api package. lens-server-api/src/main/java/org/apache/lens/server/api/query/save/SavedQueryService.java (line 29) <https://reviews.apache.org/r/37934/#comment153984> Please add javadoc for the methods. lens-server/src/main/java/org/apache/lens/server/savedquery/ListResponse.java (line 33) <https://reviews.apache.org/r/37934/#comment153985> Would move to lens-api package. lens-server/src/main/java/org/apache/lens/server/savedquery/ResourceModifiedResponse.java (line 30) <https://reviews.apache.org/r/37934/#comment153986> Would move to lens-api package lens-server/src/main/java/org/apache/lens/server/savedquery/SavedQueryDao.java (line 232) <https://reviews.apache.org/r/37934/#comment153987> I assume this is HSQL? if so, please rename to HSQL instead of HQL. lens-server/src/main/java/org/apache/lens/server/savedquery/SavedQueryDao.java (line 264) <https://reviews.apache.org/r/37934/#comment153988> what is this Result handler for? Can you add comments? lens-server/src/main/java/org/apache/lens/server/savedquery/SavedQueryResource.java (line 53) <https://reviews.apache.org/r/37934/#comment153991> should we have this under queryapi/ itself? I'm thinking 'yes'. What do you think? Making this path /queryapi will cause any issue with other query resource? I'm hoping it should not, not sure though. lens-server/src/main/java/org/apache/lens/server/savedquery/SavedQueryResource.java (line 54) <https://reviews.apache.org/r/37934/#comment153989> Please add javadoc for all methods and all params clearly, sothat resource doc gets updated in site docs. Have suggested changes in api paths. Put them below: GET on /savedqueries will give list of saved queries POST on /savedqueries will create a new saved query. GET on /savedqueries/{id} : Get saved query. DELETE on /savedqueries/{id} : Delete saved query. PUT on /savedqueries/{id} : Updated saved query. POST on /savedqueries/{id} : Will execute the saved query. lens-server/src/main/java/org/apache/lens/server/savedquery/SavedQueryResource.java (line 79) <https://reviews.apache.org/r/37934/#comment153990> list cannot be a path name should be /savedqueries lens-server/src/main/java/org/apache/lens/server/savedquery/SavedQueryResource.java (line 98) <https://reviews.apache.org/r/37934/#comment153993> let us make this path to be savedqueries/{id} lens-server/src/main/java/org/apache/lens/server/savedquery/SavedQueryResource.java (line 111) <https://reviews.apache.org/r/37934/#comment153994> let us make this path to be savedqueries/{id} lens-server/src/main/java/org/apache/lens/server/savedquery/SavedQueryResource.java (line 127) <https://reviews.apache.org/r/37934/#comment153995> There is no path specified here. It should have been savedqueries lens-server/src/main/java/org/apache/lens/server/savedquery/SavedQueryResource.java (line 147) <https://reviews.apache.org/r/37934/#comment153996> let us make this path to be savedqueries/{id} lens-server/src/main/java/org/apache/lens/server/savedquery/SavedQueryResource.java (line 173) <https://reviews.apache.org/r/37934/#comment153997> let us make this path to be savedqueries/{id}. POST on id will be executing the query. lens-server/src/main/java/org/apache/lens/server/savedquery/SavedQueryServiceImpl.java (line 46) <https://reviews.apache.org/r/37934/#comment154000> Please move this to LensConfConstants and add it in lensserver-default.xml. lens-server/src/test/java/org/apache/lens/server/query/save/TestSavedQueryService.java (line 105) <https://reviews.apache.org/r/37934/#comment154002> Please go ahead and remove @localhost, if it needs to be removed. lens-server/src/test/resources/lens-site.xml (lines 141 - 152) <https://reviews.apache.org/r/37934/#comment154003> Need to be removed? - Amareshwari Sriramadasu On Sept. 4, 2015, 5:25 p.m., Amruth Sampath wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/37934/ > ----------------------------------------------------------- > > (Updated Sept. 4, 2015, 5:25 p.m.) > > > Review request for lens, Amareshwari Sriramadasu, Pranav Agarwal, Rajat > Khandelwal, and sharad agarwal. > > > Repository: lens > > > Description > ------- > > Refer to the JIRA description - https://issues.apache.org/jira/browse/LENS-742 > (Note : Sharing and CLI are not a part of this patch) > > > Diffs > ----- > > lens-api/src/main/java/org/apache/lens/api/error/LensCommonErrorCode.java > 754e6e1 > lens-api/src/main/resources/lens-errors.conf 3fb191e > lens-cube/src/main/java/org/apache/lens/cube/parse/CubeQueryRewriter.java > 0dfd7da > lens-cube/src/main/java/org/apache/lens/cube/parse/HQLParser.java 586629f > > lens-server-api/src/main/java/org/apache/lens/server/api/query/save/Parameter.java > PRE-CREATION > > lens-server-api/src/main/java/org/apache/lens/server/api/query/save/ParameterCollectionType.java > PRE-CREATION > > lens-server-api/src/main/java/org/apache/lens/server/api/query/save/ParameterDataType.java > PRE-CREATION > > lens-server-api/src/main/java/org/apache/lens/server/api/query/save/ParameterParser.java > PRE-CREATION > > lens-server-api/src/main/java/org/apache/lens/server/api/query/save/ParameterParserResponse.java > PRE-CREATION > > lens-server-api/src/main/java/org/apache/lens/server/api/query/save/ParameterResolver.java > PRE-CREATION > > lens-server-api/src/main/java/org/apache/lens/server/api/query/save/SavedQuery.java > PRE-CREATION > > lens-server-api/src/main/java/org/apache/lens/server/api/query/save/SavedQueryService.java > PRE-CREATION > > lens-server-api/src/main/java/org/apache/lens/server/api/query/save/exception/MissingParameterException.java > PRE-CREATION > > lens-server-api/src/main/java/org/apache/lens/server/api/query/save/exception/ParameterCollectionException.java > PRE-CREATION > > lens-server-api/src/main/java/org/apache/lens/server/api/query/save/exception/ParameterValueException.java > PRE-CREATION > > lens-server-api/src/main/java/org/apache/lens/server/api/query/save/exception/PrivilegeException.java > PRE-CREATION > > lens-server-api/src/main/java/org/apache/lens/server/api/query/save/exception/SavedQueryNotFound.java > PRE-CREATION > > lens-server-api/src/main/java/org/apache/lens/server/api/query/save/exception/ValueEncodeException.java > PRE-CREATION > > lens-server-api/src/test/java/org/apache/lens/server/api/query/save/TestParameterParser.java > PRE-CREATION > > lens-server-api/src/test/java/org/apache/lens/server/api/query/save/TestParameterResolution.java > PRE-CREATION > > lens-server/src/main/java/org/apache/lens/server/savedquery/ListResponse.java > PRE-CREATION > > lens-server/src/main/java/org/apache/lens/server/savedquery/ResourceModifiedResponse.java > PRE-CREATION > > lens-server/src/main/java/org/apache/lens/server/savedquery/SavedQueryApp.java > PRE-CREATION > > lens-server/src/main/java/org/apache/lens/server/savedquery/SavedQueryDao.java > PRE-CREATION > > lens-server/src/main/java/org/apache/lens/server/savedquery/SavedQueryResource.java > PRE-CREATION > > lens-server/src/main/java/org/apache/lens/server/savedquery/SavedQueryServiceImpl.java > PRE-CREATION > lens-server/src/main/java/org/apache/lens/server/util/UtilityMethods.java > 5d77eb7 > lens-server/src/main/resources/lensserver-default.xml 5d33eda > > lens-server/src/test/java/org/apache/lens/server/query/QueryAPIErrorResponseTest.java > 2189eb8 > > lens-server/src/test/java/org/apache/lens/server/query/save/TestSavedQueryService.java > PRE-CREATION > lens-server/src/test/resources/lens-site.xml 4cf94d5 > > Diff: https://reviews.apache.org/r/37934/diff/ > > > Testing > ------- > > Have added unit test cases for parsing, resolution of parameter and service > testing. > > > Thanks, > > Amruth Sampath > >
