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




client/src/main/java/org/apache/oozie/cli/OozieCLI.java
Lines 2168-2169 (patched)
<https://reviews.apache.org/r/59620/#comment283755>

    Let's start with schema version 1.0



client/src/main/resources/git-action-0.1.xsd
Lines 26-30 (patched)
<https://reviews.apache.org/r/59620/#comment283756>

    Plese use the new oozie-common namespace to make launcher config possible 
and use resoure-manager and instead of job-tracker. Take a look at the 
spark-action-1.0.xsd for reference



client/src/main/resources/git-action-0.1.xsd
Lines 32 (patched)
<https://reviews.apache.org/r/59620/#comment283765>

    I don't know if it's possible to define uris outside 
oozie.service.HadoopAccessorService.nameNode.whitelist here.
    
    Can you please add a test for that?



sharelib/git/src/main/java/org/apache/oozie/action/hadoop/GitMain.java
Lines 56 (patched)
<https://reviews.apache.org/r/59620/#comment283757>

    I don't know if we shoudl provide compatibility to all the way back to 
mapred.job.tracker. I'd prefer to only look for yarn.resourcemanager.address.



sharelib/git/src/main/java/org/apache/oozie/action/hadoop/GitMain.java
Lines 64 (patched)
<https://reviews.apache.org/r/59620/#comment283764>

    This looks unneccessary. Where it's used?



sharelib/git/src/main/java/org/apache/oozie/action/hadoop/GitMain.java
Lines 75-82 (patched)
<https://reviews.apache.org/r/59620/#comment283759>

    This is not used anywhere if I see it correctly



sharelib/git/src/main/java/org/apache/oozie/action/hadoop/GitMain.java
Lines 137-145 (patched)
<https://reviews.apache.org/r/59620/#comment283760>

    I wonder how strict we should be with credential files. We might want to do 
a ssh-like check. Ssh fails if your .pem file is not readably only by your 
user, that's why we're setting the permissions here.
    
    At least we could print out a warning to help users avoid leaking 
credentials to everyone from HDFS
    
    Or we can go ssh-level strict and have an option to do fall back to 
warnings only.



sharelib/git/src/main/java/org/apache/oozie/action/hadoop/GitMain.java
Lines 229-236 (patched)
<https://reviews.apache.org/r/59620/#comment283763>

    Please extract a method for creating local tmp folders.


- Peter Cseh


On July 7, 2017, 1:27 a.m., Clay B. wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59620/
> -----------------------------------------------------------
> 
> (Updated July 7, 2017, 1:27 a.m.)
> 
> 
> Review request for oozie and András Piros.
> 
> 
> Bugs: OOZIE-2877
>     https://issues.apache.org/jira/browse/OOZIE-2877
> 
> 
> Repository: oozie-git
> 
> 
> Description
> -------
> 
> OOZIE-2877 - Oozie Git Action
> 
> 
> Diffs
> -----
> 
>   client/src/main/java/org/apache/oozie/cli/OozieCLI.java 4adf1a8 
>   client/src/main/resources/git-action-0.1.xsd PRE-CREATION 
>   core/src/main/java/org/apache/oozie/action/hadoop/GitActionExecutor.java 
> PRE-CREATION 
>   core/src/main/resources/oozie-default.xml 5629a89 
>   docs/src/site/twiki/WorkflowFunctionalSpec.twiki 6bd3e5a 
>   pom.xml 16c5137 
>   sharelib/git/pom.xml PRE-CREATION 
>   sharelib/git/src/main/java/org/apache/oozie/action/hadoop/GitMain.java 
> PRE-CREATION 
>   
> sharelib/git/src/test/java/org/apache/oozie/action/hadoop/TestGitActionExecutor.java
>  PRE-CREATION 
>   
> sharelib/git/src/test/java/org/apache/oozie/action/hadoop/TestGitMainGetKey.java
>  PRE-CREATION 
>   
> sharelib/git/src/test/java/org/apache/oozie/action/hadoop/TestIntegrationGitActionExecutor.java
>  PRE-CREATION 
>   sharelib/pom.xml d794246 
>   src/main/assemblies/sharelib.xml 07dc69c 
>   webapp/pom.xml ee6341c 
> 
> 
> Diff: https://reviews.apache.org/r/59620/diff/2/
> 
> 
> Testing
> -------
> 
> Tested using unit and integration tests. Still need to:
> * Test on a cluster
> * Test with an authenticated SSH hosted Git repo
> 
> Sumitted a request to the JGit community as their branch pulling code seems 
> to have an 
> [issue](https://dev.eclipse.org/mhonarc/lists/jgit-dev/msg03343.html).
> 
> 
> File Attachments
> ----------------
> 
> 0001-OOZIE-2877-Oozie-Git-Action.patch
>   
> https://reviews.apache.org/media/uploaded/files/2017/05/29/24f90a78-3dc1-49fe-bf29-5927a3cd5e72__0001-OOZIE-2877-Oozie-Git-Action.patch
> Patch
>   
> https://reviews.apache.org/media/uploaded/files/2017/05/29/dd23dd72-67e0-456f-9b52-e566d8d17d16__0001-OOZIE-2877-Oozie-Git-Action.patch
> 
> 
> Thanks,
> 
> Clay B.
> 
>

Reply via email to