-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/51964/#review155416
-----------------------------------------------------------




lens-server-api/src/main/java/org/apache/lens/server/api/LensConfConstants.java 
(lines 837 - 849)
<https://reviews.apache.org/r/51964/#comment225885>

    Should we add the new properties to default lens server config file along 
with the description on how and when to use them?



lens-server/src/main/java/org/apache/lens/server/session/DatabaseResourceService.java
 (line 84)
<https://reviews.apache.org/r/51964/#comment225881>

    [Suggestion] Should we simplify this method by assuming there will be only 
one level of folders under the base resource directory ?
    
    If we use this knowledge we can sync one DB at a time and paring the urls 
may not be required.



lens-server/src/main/java/org/apache/lens/server/session/DatabaseResourceService.java
 (line 87)
<https://reviews.apache.org/r/51964/#comment225769>

    Can we reuse "dbResourceFs" ?



lens-server/src/main/java/org/apache/lens/server/session/DatabaseResourceService.java
 (lines 97 - 109)
<https://reviews.apache.org/r/51964/#comment225886>

    Can we add log entries when jars are downloaded



lens-server/src/main/java/org/apache/lens/server/session/DatabaseResourceService.java
 (line 143)
<https://reviews.apache.org/r/51964/#comment225349>

    This needs to be moved to end so that service state is changed to STARTED 
in the end . 
    org.apache.hive.service.AbstractService#start



lens-server/src/main/java/org/apache/lens/server/session/DatabaseResourceService.java
 (lines 157 - 159)
<https://reviews.apache.org/r/51964/#comment225350>

    What will happen in case the scheme is not hdfs? 
    Will the flow work gracefully or throw an execption



lens-server/src/main/java/org/apache/lens/server/session/DatabaseResourceService.java
 (lines 161 - 163)
<https://reviews.apache.org/r/51964/#comment225890>

    What do we do for use cases where the user does not need any DB resources ? 
In those cases it may not be an ERROR to not have top resource directory



lens-server/src/main/java/org/apache/lens/server/session/DatabaseResourceService.java
 (lines 193 - 194)
<https://reviews.apache.org/r/51964/#comment225888>

    Can we set dbLocalResTopDir to dbResTopDir when dbResTopDir is local path. 
This check would not be required then. 
    
    OR
    
    If needed we can also store the mode in start method as a boolean 
(local/hdfs) locally and use it in mapCommonResourceEntries(boolean mode) and 
mapDbResourceEntries(boolean mode)



lens-server/src/main/java/org/apache/lens/server/session/DatabaseResourceService.java
 (lines 210 - 211)
<https://reviews.apache.org/r/51964/#comment225893>

    This will never be true since the DB path is got by listing the base 
directory.



lens-server/src/main/java/org/apache/lens/server/session/DatabaseResourceService.java
 (line 231)
<https://reviews.apache.org/r/51964/#comment225889>

    Can we create the loacl fileSystem object only once (may be in start() 
method and) use it elsewhere as hadoop FileSystem object is a heavy and 
mainatins state/cache inside it.



lens-server/src/main/java/org/apache/lens/server/session/DatabaseResourceService.java
 (lines 233 - 237)
<https://reviews.apache.org/r/51964/#comment225892>

    Isn't this condition already checked in the calling method ?



lens-server/src/main/java/org/apache/lens/server/session/DatabaseResourceService.java
 (line 265)
<https://reviews.apache.org/r/51964/#comment225984>

    Can we just retun the jar path/name or null in case there is no jar in the 
folder and shouldIncrVersion = false



lens-server/src/main/java/org/apache/lens/server/session/DatabaseResourceService.java
 (line 272)
<https://reviews.apache.org/r/51964/#comment225980>

    Can we reuase FileSystem instance here too ?



lens-server/src/main/java/org/apache/lens/server/session/DatabaseResourceService.java
 (line 278)
<https://reviews.apache.org/r/51964/#comment225981>

    Should this be >=1 ?



lens-server/src/main/java/org/apache/lens/server/session/DatabaseResourceService.java
 (line 446)
<https://reviews.apache.org/r/51964/#comment225985>

    Should we use LensConfConstants.DB_RESOURCE_COPY_FROM_HDFS property to 
figure out HDFS vs Local ? 
    
    We can rename the property to DB_RESOURCES_IN_HDFS = true/false?


This review only covers the startup flow. addDBJar Flow Review is pending, will 
review it soon.

- Puneet Gupta


On Nov. 8, 2016, 10:18 a.m., Sushil Mohanty wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51964/
> -----------------------------------------------------------
> 
> (Updated Nov. 8, 2016, 10:18 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/pom.xml 8e5e3eb 
>   
> lens-cli/src/main/java/org/apache/lens/cli/commands/LensDatabaseCommands.java 
> c6ae02b 
>   lens-cli/src/test/java/org/apache/lens/cli/TestLensDatabaseCommands.java 
> 7fc8438 
>   lens-cli/src/test/resources/schema/jars/test_db_resource.jar PRE-CREATION 
>   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
>  8f9db2a 
>   
> lens-server-api/src/main/java/org/apache/lens/server/api/metastore/CubeMetastoreService.java
>  28b9d22 
>   
> lens-server-api/src/main/java/org/apache/lens/server/api/session/SessionService.java
>  20ec686 
>   lens-server/pom.xml d24dc1e 
>   
> lens-server/src/main/java/org/apache/lens/server/metastore/CubeMetastoreServiceImpl.java
>  8b10d1d 
>   
> lens-server/src/main/java/org/apache/lens/server/session/DatabaseResourceService.java
>  511e4cf 
>   
> lens-server/src/main/java/org/apache/lens/server/session/HiveSessionService.java
>  21e2a62 
>   
> lens-server/src/main/java/org/apache/lens/server/session/LensSessionImpl.java 
> 34c901c 
>   
> lens-server/src/main/java/org/apache/lens/server/session/SessionResource.java 
> 63eea63 
>   lens-server/src/main/java/org/apache/lens/server/util/ScannedPaths.java 
> e48eab4 
>   lens-server/src/main/resources/lensserver-default.xml a00048b 
>   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/session/TestDatabaseResourceService.java
>  2bc3712 
>   
> lens-server/src/test/java/org/apache/lens/server/session/TestDatabaseService.java
>  PRE-CREATION 
>   
> lens-server/src/test/java/org/apache/lens/server/session/TestSessionClassLoaders.java
>  d66de4c 
>   lens-server/src/test/resources/lens-site.xml d96659f 
>   pom.xml 29c59d3 
> 
> 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