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

Reply via email to