----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51964/#review152818 -----------------------------------------------------------
lens-cli/src/main/java/org/apache/lens/cli/commands/LensDatabaseCommands.java (line 134) <https://reviews.apache.org/r/51964/#comment222229> How are we handling HDFS path ? are we downloading this jar to local machine (hosting the cli) and then uploading to server ? lens-client/src/main/java/org/apache/lens/client/LensConnection.java (lines 393 - 396) <https://reviews.apache.org/r/51964/#comment222230> How do we handle non jar type resources both on client and on server ? lens-server/src/main/java/org/apache/lens/server/metastore/CubeMetastoreServiceImpl.java (line 738) <https://reviews.apache.org/r/51964/#comment222046> Should we have logs statements at the start and end of this method as its syncronized so that we know why the other addjar call is waiting (if that situation ever arises) lens-server/src/main/java/org/apache/lens/server/metastore/CubeMetastoreServiceImpl.java (line 760) <https://reviews.apache.org/r/51964/#comment222225> Do we need to update org.apache.lens.server.session.DatabaseResourceService#dbResEntryMap with entry in this case? lens-server/src/main/java/org/apache/lens/server/metastore/CubeMetastoreServiceImpl.java (lines 762 - 764) <https://reviews.apache.org/r/51964/#comment222228> How is addJar updating org.apache.lens.server.session.DatabaseResourceService#classLoaderCache with calssloader pointing to the newly uploaded jar? Will the new jar be refletced in exsitin session calss loader as well ? lens-server/src/main/java/org/apache/lens/server/metastore/CubeMetastoreServiceImpl.java (lines 792 - 795) <https://reviews.apache.org/r/51964/#comment222042> Should we fail in this case or create the directory and add the jar? I feel, the use case where a user creates a new DB from lens CLI and then uploads a jar should work without admin intervention. lens-server/src/main/java/org/apache/lens/server/metastore/CubeMetastoreServiceImpl.java (line 801) <https://reviews.apache.org/r/51964/#comment222044> Should we show a different message to user which says "This database {dbname} does not support jar upload" The warning message can still have details that jar_oder file is present and hence upload is not allowed lens-server/src/main/java/org/apache/lens/server/metastore/CubeMetastoreServiceImpl.java (lines 808 - 812) <https://reviews.apache.org/r/51964/#comment222048> Theoratically this case should not arive since the method is syncronized. If this does occur, it can be because server was resttarted while a jar was still uploading or rename opertaion failed. In this case(s) we should overwrite the jar with a warning message. Please check once. lens-server/src/main/java/org/apache/lens/server/metastore/CubeMetastoreServiceImpl.java (lines 815 - 827) <https://reviews.apache.org/r/51964/#comment222204> should we move this logic to a utility and use it for both DatabaseResourceService and CubeMetastoreServiceImpl ? lens-server/src/main/java/org/apache/lens/server/metastore/CubeMetastoreServiceImpl.java (line 829) <https://reviews.apache.org/r/51964/#comment222205> This seems same as "uploadingPath" lens-server/src/main/java/org/apache/lens/server/metastore/CubeMetastoreServiceImpl.java (line 834) <https://reviews.apache.org/r/51964/#comment222211> Can we resuse "dbDir" in all paces where DB path is required lens-server/src/main/java/org/apache/lens/server/metastore/CubeMetastoreServiceImpl.java (lines 840 - 845) <https://reviews.apache.org/r/51964/#comment222208> Should we club the two exceptions (catch with multiple exceptions) and just say "Execption while uploading jar". The stack trace will have the exception type and details anyway. Or we can even have smaller try blocks and print the exact execption message (Say a try block around IOUtils.copy(is, fos) and the catch says execption wile copying jar) lens-server/src/main/java/org/apache/lens/server/metastore/CubeMetastoreServiceImpl.java (line 1397) <https://reviews.apache.org/r/51964/#comment222216> Are two commands required for Hive ? lens-server/src/main/java/org/apache/lens/server/metastore/MetastoreResource.java (lines 1566 - 1567) <https://reviews.apache.org/r/51964/#comment222217> fileDetail needs to me added too ? lens-server/src/main/java/org/apache/lens/server/metastore/MetastoreResource.java (line 1576) <https://reviews.apache.org/r/51964/#comment222219> can only JARs be added as resources ? If yes then should we rename the method to include jar lens-server/src/main/java/org/apache/lens/server/metastore/MetastoreResource.java (line 1581) <https://reviews.apache.org/r/51964/#comment222232> This file size is passed by the client and can be dummy size. Do we need to reconfirm theactual size on server (This can be done while copying jar to local)? lens-server/src/main/java/org/apache/lens/server/metastore/MetastoreResource.java (lines 1582 - 1584) <https://reviews.apache.org/r/51964/#comment222231> Space required before FileSize and AllowedSize lens-server/src/main/java/org/apache/lens/server/session/DatabaseResourceService.java (line 51) <https://reviews.apache.org/r/51964/#comment221940> can we update the variable name to match the ds lens-server/src/main/java/org/apache/lens/server/session/DatabaseResourceService.java (line 84) <https://reviews.apache.org/r/51964/#comment221943> lens-server/src/main/java/org/apache/lens/server/session/DatabaseResourceService.java (line 91) <https://reviews.apache.org/r/51964/#comment221947> should we call this in the end ? I feel, the INITED state can be flagged after local initialization. lens-server/src/main/java/org/apache/lens/server/session/DatabaseResourceService.java (lines 113 - 116) <https://reviews.apache.org/r/51964/#comment221950> else part can be skipped. resTopDir is already initialized. If it makes sense , we can move this initialization (resTopDir = getHiveConf().get(LensConfConstants.DATABASE_RESOURCE_DIR...)) to init() method also. lens-server/src/main/java/org/apache/lens/server/session/DatabaseResourceService.java (line 121) <https://reviews.apache.org/r/51964/#comment221977> Should we change this to "DB resources base location does not exist" ? lens-server/src/main/java/org/apache/lens/server/session/DatabaseResourceService.java (line 125) <https://reviews.apache.org/r/51964/#comment221978> Should we chnage the message to "Error locating DB resources base directory" ? lens-server/src/main/java/org/apache/lens/server/session/DatabaseResourceService.java (line 192) <https://reviews.apache.org/r/51964/#comment222001> can we log "dbResEntryMap.get(dbname)" instaed lens-server/src/main/java/org/apache/lens/server/session/DatabaseResourceService.java (line 295) <https://reviews.apache.org/r/51964/#comment222037> can we also add DB name in error message lens-server/src/main/java/org/apache/lens/server/session/DatabaseResourceService.java (line 415) <https://reviews.apache.org/r/51964/#comment222223> Should we remove Map from method name lens-server/src/main/java/org/apache/lens/server/session/LensSessionImpl.java (line 429) <https://reviews.apache.org/r/51964/#comment222222> When is readClassLoaderFromCache false ? - Puneet Gupta On Oct. 14, 2016, 8:29 a.m., Sushil Mohanty wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/51964/ > ----------------------------------------------------------- > > (Updated Oct. 14, 2016, 8:29 a.m.) > > > Review request for lens. > > > Bugs: LENS-317 > https://issues.apache.org/jira/browse/LENS-317 > > > Repository: lens > > > Description > ------- > > Server side api call to update database jar without restarting lens server. > More details can be found in LENS-317. > > > Diffs > ----- > > > lens-cli/src/main/java/org/apache/lens/cli/commands/LensDatabaseCommands.java > c6ae02b > lens-client/src/main/java/org/apache/lens/client/LensClient.java e936798 > lens-client/src/main/java/org/apache/lens/client/LensConnection.java > bb15b23 > > lens-server-api/src/main/java/org/apache/lens/server/api/LensConfConstants.java > 8cf617b > > lens-server-api/src/main/java/org/apache/lens/server/api/metastore/CubeMetastoreService.java > 28b9d22 > lens-server/pom.xml 6dea9a7 > > lens-server/src/main/java/org/apache/lens/server/metastore/CubeMetastoreServiceImpl.java > 8b10d1d > > lens-server/src/main/java/org/apache/lens/server/metastore/MetastoreResource.java > 9d823da > > lens-server/src/main/java/org/apache/lens/server/session/DatabaseResourceService.java > 511e4cf > > lens-server/src/main/java/org/apache/lens/server/session/LensSessionImpl.java > 34c901c > lens-server/src/main/java/org/apache/lens/server/util/ScannedPaths.java > e48eab4 > lens-server/src/test/java/org/apache/lens/server/LensJerseyTest.java > 7cccf30 > lens-server/src/test/java/org/apache/lens/server/LensServerTestUtil.java > 67cee57 > > lens-server/src/test/java/org/apache/lens/server/metastore/TestDatabaseService.java > PRE-CREATION > > lens-server/src/test/java/org/apache/lens/server/session/TestDatabaseResourceService.java > 2bc3712 > lens-server/src/test/resources/lens-site.xml d96659f > > Diff: https://reviews.apache.org/r/51964/diff/ > > > Testing > ------- > > mvn clean install. > > [INFO] > ------------------------------------------------------------------------ > [INFO] Reactor Summary: > [INFO] > [INFO] Lens Checkstyle Rules ............................. SUCCESS [3.302s] > [INFO] Lens .............................................. SUCCESS [7.286s] > [INFO] Lens API .......................................... SUCCESS [31.546s] > [INFO] Lens API for server and extensions ................ SUCCESS [25.681s] > [INFO] Lens Cube ......................................... SUCCESS > [17:55.255s] > [INFO] Lens DB storage ................................... SUCCESS [25.650s] > [INFO] Lens Query Library ................................ SUCCESS [21.646s] > [INFO] Lens Hive Driver .................................. SUCCESS [2:11.167s] > [INFO] Lens Driver for JDBC .............................. SUCCESS [1:03.464s] > [INFO] Lens Elastic Search Driver ........................ SUCCESS [54.798s] > [INFO] Lens Server ....................................... SUCCESS > [18:19.588s] > [INFO] Lens client ....................................... SUCCESS [2:02.590s] > [INFO] Lens CLI .......................................... SUCCESS [1:54.985s] > [INFO] Lens Examples ..................................... SUCCESS [13.790s] > [INFO] Lens Ship Jars to Distributed Cache ............... SUCCESS [2.061s] > [INFO] Lens Distribution ................................. SUCCESS [24.869s] > [INFO] Lens ML Lib ....................................... SUCCESS [1:54.426s] > [INFO] Lens ML Ext Distribution .......................... SUCCESS [11.053s] > [INFO] Lens Regression ................................... SUCCESS [20.677s] > [INFO] Lens UI ........................................... SUCCESS [19.454s] > [INFO] > ------------------------------------------------------------------------ > [INFO] BUILD SUCCESS > [INFO] > ------------------------------------------------------------------------ > [INFO] Total time: 49:44.002s > [INFO] Finished at: Sat Sep 17 00:12:08 IST 2016 > [INFO] Final Memory: 178M/2490M > [INFO] > ------------------------------------------------------------------------ > > > Thanks, > > Sushil Mohanty > >
