> On June 19, 2015, 6:25 a.m., Rajat Khandelwal wrote: > > lens-cli/src/test/java/org/apache/lens/cli/ExecuteQueryCommandIT.java, line > > 28 > > <https://reviews.apache.org/r/35602/diff/1/?file=986827#file986827line28> > > > > Can you document this class?
Its an integration test class. The test case is small and Identifier names are self documenting. Please provide more specifics on the type of documentation required. > On June 19, 2015, 6:25 a.m., Rajat Khandelwal wrote: > > lens-client/src/main/java/org/apache/lens/client/exceptions/LensClientAsyncQueryFailedException.java, > > line 29 > > <https://reviews.apache.org/r/35602/diff/1/?file=986832#file986832line29> > > > > final? Added > On June 19, 2015, 6:25 a.m., Rajat Khandelwal wrote: > > lens-client/src/main/java/org/apache/lens/client/exceptions/LensClientAsyncQueryFailedException.java, > > line 31 > > <https://reviews.apache.org/r/35602/diff/1/?file=986832#file986832line31> > > > > Can be replaced by @AllArgsConstructor That can be done in this case with a @NonNull on errorResult field. However, there is no @NotBlank annotation in lombok, so the moment a new field is added in class with type as String, a checkArgument(StringUtils.isNotBlank(stringField)) is left out due to honest mistakes. To avoid writing code which has chances of going bad, @AllArgsConstructor is avoided here. > On June 19, 2015, 6:25 a.m., Rajat Khandelwal wrote: > > lens-client/src/main/java/org/apache/lens/client/exceptions/LensClientException.java, > > line 59 > > <https://reviews.apache.org/r/35602/diff/1/?file=986833#file986833line59> > > > > Should we keep a cause argument here as well? Wouldn't this be sufficient ? http://docs.oracle.com/javase/7/docs/api/java/lang/Throwable.html#getCause() > On June 19, 2015, 6:25 a.m., Rajat Khandelwal wrote: > > lens-client/src/main/java/org/apache/lens/client/exceptions/LensClientException.java, > > line 68 > > <https://reviews.apache.org/r/35602/diff/1/?file=986833#file986833line68> > > > > Instead of creating a new method, let's override getMessage to return > > this message or super.getMessage() Lets try to see it this way. Code interacting with LensClientException is interested in getting the error message returned by Lens APIs. Intuition would ask for a method which is self documenting to do the same. That method is getLensAPIErrorMessage. Intuition is aligned to the thought that Throwable.getMessage is going to return the message with which exception was constructed, not the exception returned by Lens APIs. - Himanshu ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35602/#review88491 ----------------------------------------------------------- On June 18, 2015, 1:03 p.m., Himanshu Gahlaut wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/35602/ > ----------------------------------------------------------- > > (Updated June 18, 2015, 1:03 p.m.) > > > Review request for lens. > > > Bugs: LENS-487 > https://issues.apache.org/jira/browse/LENS-487 > > > Repository: lens > > > Description > ------- > > LENS-487: Implementation for consistent error response display for Lens Client > > > Diffs > ----- > > lens-api/src/main/java/org/apache/lens/api/query/LensQuery.java > c22d39a38b0d7bc68d89fdcc0aae06d4c4449553 > lens-api/src/main/java/org/apache/lens/api/query/QueryStatus.java > f92737507faacc5e7f3d76f4fde29fdd445bfed7 > lens-api/src/main/java/org/apache/lens/api/response/LensErrorTO.java > 313566c9cdf10fe10a044b125f0fbdd5fd47678d > > lens-api/src/main/java/org/apache/lens/api/response/LensJAXBContextResolver.java > a401b5326920a4716a3067075a637f5f76771f23 > lens-api/src/main/java/org/apache/lens/api/response/LensResponse.java > 818ae406e847c3855da4b235f1f2f40bb5b77c72 > lens-api/src/main/java/org/apache/lens/api/response/NoErrorPayload.java > afc1a0544a00b17b2648ce43ba1d3be2544d6ebc > > lens-api/src/main/java/org/apache/lens/api/response/NoSuccessResponseData.java > 17755845358d3b18143a0d93dff5eb4073f7e336 > lens-api/src/main/java/org/apache/lens/api/result/PrettyPrintable.java > PRE-CREATION > lens-api/src/main/resources/lens-errors.conf > 26bda1f1a5814b75b65dfe2b80f5489196cc6921 > lens-cli/src/main/java/org/apache/lens/cli/commands/LensQueryCommands.java > 928531e5bc1b55f710bfaa80949dc3ca2b9c54b4 > lens-cli/src/test/java/org/apache/lens/cli/ExecuteQueryCommandIT.java > PRE-CREATION > lens-cli/src/test/java/org/apache/lens/cli/TestLensQueryCommands.java > 97f0cf098a21eb052c5c793793cf9d230301b01d > lens-client/pom.xml e27a6cedafcb79341af40f6e33009663221c6971 > lens-client/src/main/java/org/apache/lens/client/LensClient.java > 7399d9e27bd68d1944b49c024441483b12412091 > lens-client/src/main/java/org/apache/lens/client/LensStatement.java > 40e2b8617f611deda228f2f1c70e7f769472120e > > lens-client/src/main/java/org/apache/lens/client/exceptions/LensClientAsyncQueryFailedException.java > PRE-CREATION > > lens-client/src/main/java/org/apache/lens/client/exceptions/LensClientException.java > 1150726e69f15be111683fabdd15272076ad1f1e > lens-client/src/main/java/org/apache/lens/client/model/BriefError.java > PRE-CREATION > > lens-client/src/main/java/org/apache/lens/client/model/IdBriefErrorTemplate.java > PRE-CREATION > > lens-client/src/main/java/org/apache/lens/client/model/IdBriefErrorTemplateKey.java > PRE-CREATION > lens-client/src/test/java/org/apache/lens/client/model/BriefErrorTest.java > PRE-CREATION > > lens-client/src/test/java/org/apache/lens/client/model/IdBriefErrorTemplateTest.java > PRE-CREATION > > lens-cube/src/main/java/org/apache/lens/cube/error/ColUnAvailableInTimeRangeException.java > 99a78f69479a353f1b34b13a187ac590213821c1 > > lens-cube/src/main/java/org/apache/lens/cube/error/FieldsCannotBeQueriedTogetherException.java > f24624665832bc6f7948577454016d8e8c1e915d > lens-examples/src/main/java/org/apache/lens/examples/SampleQueries.java > 6c820a46c89a9638145cf631b7d022a86216a171 > lens-ml-lib/src/main/java/org/apache/lens/ml/impl/LensMLImpl.java > de76603fa8ab5730d8384e7ac04bc29b9f87244f > lens-ml-lib/src/main/java/org/apache/lens/rdd/LensRDDClient.java > fe4d9267db0b4f0bb8882618c0275e41027d7ce3 > > lens-regression/src/main/java/org/apache/lens/regression/core/helpers/QueryHelper.java > 6b8bdda7a39f8064fca5e8914492f691a335c3af > > lens-server-api/src/main/java/org/apache/lens/server/api/driver/DriverQueryStatus.java > 2c3527674ec90a0a260a7ed13b77320ed94e2250 > > lens-server-api/src/main/java/org/apache/lens/server/api/error/LensException.java > 9d8748c0734b9c3f99dd9c7daabd27eb88be4973 > > lens-server-api/src/main/java/org/apache/lens/server/api/error/LensMultiCauseException.java > eb0df4de51e76da6470b4b916f40883ca3d3a116 > > lens-server-api/src/main/java/org/apache/lens/server/api/query/FinishedLensQuery.java > 9ba18434ca630a0ab5ff9428b5ad977a3d62afe7 > > lens-server-api/src/main/java/org/apache/lens/server/api/query/QueryContext.java > 3e0c26c966ee37db8ce168fbacaa792b35a83d77 > lens-server/src/main/java/org/apache/lens/server/LensServer.java > e200bdfa52dfdc0715f6b2c9dc53468abed980fa > > lens-server/src/main/java/org/apache/lens/server/error/LensExceptionMapper.java > 7240a648ecf4a623f9543dc9b8e1cb3afb7342d5 > > lens-server/src/main/java/org/apache/lens/server/error/UnSupportedQuerySubmitOpException.java > f0a0ab0ec5da3fc15698de3d03fba202ce898c53 > > lens-server/src/main/java/org/apache/lens/server/query/QueryExecutionServiceImpl.java > e41a0d28709d8d327994c7ccc57d535205c30260 > > lens-server/src/main/java/org/apache/lens/server/query/QueryServiceResource.java > badde8c4bb219a17df9be8064ce004c165bdd49e > lens-server/src/main/java/org/apache/lens/server/query/ResultFormatter.java > de7e1201dbffbf662eb8337d73b627597110511c > > lens-server/src/main/java/org/apache/lens/server/query/SupportedQuerySubmitOperations.java > 50858a683f419510ce6ae4eb77ac7cf585ab1388 > lens-server/src/test/java/org/apache/lens/server/LensTestUtil.java > c05a2e772233427e6965e0724aac70fe48c6695a > lens-server/src/test/java/org/apache/lens/server/TestServerMode.java > b33b9069378b3de83b4377fc772c7276bae1a3ac > lens-server/src/test/java/org/apache/lens/server/TestServerRestart.java > 09f8dc71a89559d99ffd1cb9731b678618116da4 > > lens-server/src/test/java/org/apache/lens/server/common/ErrorResponseExpectedData.java > 3963a06b1c82eacc5c7e928fe80663a0e60d23d6 > lens-server/src/test/java/org/apache/lens/server/common/TestDataUtils.java > a59252c5d7085832d3f62c99b0ea9cec21aad337 > > lens-server/src/test/java/org/apache/lens/server/metrics/TestResourceMethodMetrics.java > 695403e443d99fbd3f057758d1bc7e5af432d207 > > lens-server/src/test/java/org/apache/lens/server/query/QueryAPIErrorResponseTest.java > 4c2a7a4849d843914a4ff535b0de888ea20dee11 > > lens-server/src/test/java/org/apache/lens/server/query/TestQueryEndEmailNotifier.java > d3fa02ab230a6cf4918bb17b7f535a1bbd4b6bf1 > > lens-server/src/test/java/org/apache/lens/server/query/TestQueryService.java > 62c24ba21ff9a202c7843962625feef72a867d26 > > lens-server/src/test/java/org/apache/lens/server/query/TestResultFormatting.java > f65805d02fc0a3124672ae460bface8c8bc700bb > pom.xml df0b76659f0ed6c36a63a324105414739fe21269 > > Diff: https://reviews.apache.org/r/35602/diff/ > > > Testing > ------- > > New test cases added. All existing test cases passed. > > [INFO] Reactor Summary: > [INFO] > [INFO] Lens Checkstyle Rules ............................. SUCCESS [17.925s] > [INFO] Lens .............................................. SUCCESS [13.027s] > [INFO] Lens API .......................................... SUCCESS [49.526s] > [INFO] Lens API for server and extensions ................ SUCCESS [49.076s] > [INFO] Lens Cube ......................................... SUCCESS [5:08.265s] > [INFO] Lens DB storage ................................... SUCCESS [25.491s] > [INFO] Lens Query Library ................................ SUCCESS [21.368s] > [INFO] Lens Hive Driver .................................. SUCCESS [3:50.910s] > [INFO] Lens Driver for JDBC .............................. SUCCESS [48.627s] > [INFO] Lens Server ....................................... SUCCESS > [12:37.530s] > [INFO] Lens client ....................................... SUCCESS [54.665s] > [INFO] Lens CLI .......................................... SUCCESS [4:09.569s] > [INFO] Lens Examples ..................................... SUCCESS [15.038s] > [INFO] Lens Distribution ................................. SUCCESS [14.444s] > [INFO] Lens ML Lib ....................................... SUCCESS [2:36.555s] > [INFO] Lens ML Ext Distribution .......................... SUCCESS [13.506s] > [INFO] Lens Regression ................................... SUCCESS [18.322s] > [INFO] > ------------------------------------------------------------------------ > [INFO] BUILD SUCCESS > [INFO] > ------------------------------------------------------------------------ > [INFO] Total time: 34:05.023s > [INFO] Finished at: Thu Jun 18 18:23:03 IST 2015 > [INFO] Final Memory: 118M/363M > [INFO] > ------------------------------------------------------------------------ > > > Thanks, > > Himanshu Gahlaut > >
