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



core/src/main/java/org/apache/oozie/action/hadoop/JavaActionExecutor.java
<https://reviews.apache.org/r/27118/#comment100331>

    Call it HadoopShims.class. Just for the sake of sticking to the same naming 
convention used by other projects (pig, hive, hbase, etc) when they deal with 
multiple versions of hadoop. Easy to look for by folks familiar with other 
projects.



core/src/main/java/org/apache/oozie/service/ShareLibService.java
<https://reviews.apache.org/r/27118/#comment100332>

    unused import



core/src/main/java/org/apache/oozie/service/ShareLibService.java
<https://reviews.apache.org/r/27118/#comment100356>

    Add a comment saying
    
    // Map<sharelibname, Map<symlink, symlinktarget>>



core/src/main/java/org/apache/oozie/service/ShareLibService.java
<https://reviews.apache.org/r/27118/#comment100363>

    If it is a file, it is not being added to sharelib now.



core/src/main/java/org/apache/oozie/service/ShareLibService.java
<https://reviews.apache.org/r/27118/#comment100347>

    Why create so many instances of new HadoopFileSystem(fs). Can instantiate 
once after fs instantiation.
    
    Just log if the file is a symlink with the information that it is a 
symlink. Currently only path space true/false is printed. Also you can just log 
once for the rootDir instead of every file.



core/src/main/java/org/apache/oozie/service/ShareLibService.java
<https://reviews.apache.org/r/27118/#comment100350>

    private



core/src/main/java/org/apache/oozie/service/ShareLibService.java
<https://reviews.apache.org/r/27118/#comment100358>

    This comment is wrong. Should be for getSystemLibJars method



core/src/main/java/org/apache/oozie/service/ShareLibService.java
<https://reviews.apache.org/r/27118/#comment100360>

    sharelibName instead of actionKey



core/src/main/java/org/apache/oozie/service/ShareLibService.java
<https://reviews.apache.org/r/27118/#comment100361>

    sharelibName instead of actionKey



core/src/main/java/org/apache/oozie/service/ShareLibService.java
<https://reviews.apache.org/r/27118/#comment100351>

    Why would fs be null?



core/src/main/java/org/apache/oozie/service/ShareLibService.java
<https://reviews.apache.org/r/27118/#comment100352>

    Symlink target of [{0}] is changed



core/src/main/java/org/apache/oozie/service/ShareLibService.java
<https://reviews.apache.org/r/27118/#comment100364>

    You should just reload for that particular sharelib name instead of 
reloading the whole sharelib or meta file again. That will involve keeping the 
contents of the metafile also in memory. Do not want to reload the metafile 
unless a sharelib update command is issued.



core/src/main/java/org/apache/oozie/service/ShareLibService.java
<https://reviews.apache.org/r/27118/#comment100365>

    shareLibSymlinkMapping



core/src/test/java/org/apache/oozie/service/TestShareLibService.java
<https://reviews.apache.org/r/27118/#comment100368>

    %s/sysmlink/symlink/g



core/src/test/java/org/apache/oozie/service/TestShareLibService.java
<https://reviews.apache.org/r/27118/#comment100369>

    new HadoopFileSystem(fs) - instantiate once



core/src/test/java/org/apache/oozie/service/TestShareLibService.java
<https://reviews.apache.org/r/27118/#comment100372>

    createTestShareLibMetaFile



hadooplibs/hadoop-utils-2/src/main/java/org/apache/oozie/hadoop/utils/HadoopFileSystem.java
<https://reviews.apache.org/r/27118/#comment100354>

    getSymLinkTarget()



hadooplibs/hadoop-utils-2/src/main/java/org/apache/oozie/hadoop/utils/HadoopFileSystem.java
<https://reviews.apache.org/r/27118/#comment100355>

    annotate with @VisibleForTesting in all the classes


- Rohini Palaniswamy


On Oct. 24, 2014, 5:45 p.m., Purshotam Shah wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/27118/
> -----------------------------------------------------------
> 
> (Updated Oct. 24, 2014, 5:45 p.m.)
> 
> 
> Review request for oozie.
> 
> 
> Bugs: OOZIE-2045
>     https://issues.apache.org/jira/browse/OOZIE-2045
> 
> 
> Repository: oozie-git
> 
> 
> Description
> -------
> 
> OOZIE-2045 Symlink support for sharelib
> 
> 
> Diffs
> -----
> 
>   core/src/main/java/org/apache/oozie/action/hadoop/JavaActionExecutor.java 
> 201cfa3 
>   core/src/main/java/org/apache/oozie/service/ShareLibService.java 63c5a41 
>   core/src/main/resources/oozie-default.xml 26eb7e0 
>   core/src/test/java/org/apache/oozie/service/TestShareLibService.java 
> 68cb357 
>   
> hadooplibs/hadoop-utils-0.23/src/main/java/org/apache/oozie/hadoop/utils/HadoopFileSystem.java
>  e69de29 
>   hadooplibs/hadoop-utils-1/src/main/java/org/apache/oozie/hadoop/.DS_Store 
> e69de29 
>   
> hadooplibs/hadoop-utils-1/src/main/java/org/apache/oozie/hadoop/utils/.DS_Store
>  e69de29 
>   
> hadooplibs/hadoop-utils-1/src/main/java/org/apache/oozie/hadoop/utils/HadoopFileSystem.java
>  e69de29 
>   
> hadooplibs/hadoop-utils-2/src/main/java/org/apache/oozie/hadoop/utils/HadoopFileSystem.java
>  e69de29 
>   
> hadooplibs/hadoop-utils-3/src/main/java/org/apache/oozie/hadoop/utils/HadoopFileSystem.java
>  e69de29 
> 
> Diff: https://reviews.apache.org/r/27118/diff/
> 
> 
> Testing
> -------
> 
> UTC and manul testing
> 
> 
> Thanks,
> 
> Purshotam Shah
> 
>

Reply via email to