> On April 30, 2018, 3:12 p.m., Peter Cseh wrote: > > client/src/main/resources/git-action-0.1.xsd > > Lines 26-30 (patched) > > <https://reviews.apache.org/r/59620/diff/2/?file=1770917#file1770917line26> > > > > 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
Andras has completed this making it look like the Spark XSD. The common XSD is a really nice building block! > On April 30, 2018, 3:12 p.m., Peter Cseh wrote: > > client/src/main/resources/git-action-0.1.xsd > > Lines 32 (patched) > > <https://reviews.apache.org/r/59620/diff/2/?file=1770917#file1770917line32> > > > > 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? Thanks Peter for thinking through this. I do not know that it be necessary we allow URI's other than the HadoopAccessorService whitelisted nameservices? Andras had a request that I rename `cloneRepoToHdfs` to be less opinionated but I presumed it would still be a whitelisted HCFS of some sort. For a test here, are you thinking of me using a `file://` path for `testWhenRepoIsClonedThenGitIndexContentIsReadSuccessfully()` in `TestIntegrationGitActionExecutor.java` or can you provide me a bit more guidance or the use-case? > On April 30, 2018, 3:12 p.m., Peter Cseh wrote: > > sharelib/git/src/main/java/org/apache/oozie/action/hadoop/GitMain.java > > Lines 56 (patched) > > <https://reviews.apache.org/r/59620/diff/2/?file=1770923#file1770923line56> > > > > 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. I am sold; removed. > On April 30, 2018, 3:12 p.m., Peter Cseh wrote: > > sharelib/git/src/main/java/org/apache/oozie/action/hadoop/GitMain.java > > Lines 64 (patched) > > <https://reviews.apache.org/r/59620/diff/2/?file=1770923#file1770923line64> > > > > This looks unneccessary. Where it's used? Andras removed this dead code it appears. > On April 30, 2018, 3:12 p.m., Peter Cseh wrote: > > sharelib/git/src/main/java/org/apache/oozie/action/hadoop/GitMain.java > > Lines 75-82 (patched) > > <https://reviews.apache.org/r/59620/diff/2/?file=1770923#file1770923line75> > > > > This is not used anywhere if I see it correctly Andras removed this dead code too. > On April 30, 2018, 3:12 p.m., Peter Cseh wrote: > > sharelib/git/src/main/java/org/apache/oozie/action/hadoop/GitMain.java > > Lines 137-145 (patched) > > <https://reviews.apache.org/r/59620/diff/2/?file=1770923#file1770923line137> > > > > 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. Good idea! Indeed, I've had a hard time getting users to "do the right thing" in the past. As you see many more environments than I do, would you recommend we also check HDFS extended ACLs as well if available? Would you want an "allow insecure credential" boolean or would you want something more for the override? > On April 30, 2018, 3:12 p.m., Peter Cseh wrote: > > sharelib/git/src/main/java/org/apache/oozie/action/hadoop/GitMain.java > > Lines 229-236 (patched) > > <https://reviews.apache.org/r/59620/diff/2/?file=1770923#file1770923line229> > > > > Please extract a method for creating local tmp folders. Good idea. Also, I was not cleaning up the temporary directories it looks like. - Clay ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/59620/#review202124 ----------------------------------------------------------- On June 26, 2018, 9:50 p.m., Clay B. wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/59620/ > ----------------------------------------------------------- > > (Updated June 26, 2018, 9:50 p.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/resources/git-action-1.0.xsd PRE-CREATION > core/src/main/java/org/apache/oozie/action/hadoop/GitActionExecutor.java > PRE-CREATION > core/src/main/resources/oozie-default.xml ff1820c > core/src/test/java/org/apache/oozie/test/XTestCase.java 661970d > docs/src/site/twiki/WorkflowFunctionalSpec.twiki 76cbe21 > examples/src/main/apps/git/job.properties PRE-CREATION > examples/src/main/apps/git/workflow.xml PRE-CREATION > pom.xml 0c39d64 > sharelib/git/pom.xml PRE-CREATION > sharelib/git/src/main/java/org/apache/oozie/action/hadoop/GitMain.java > PRE-CREATION > > sharelib/git/src/main/java/org/apache/oozie/action/hadoop/GitOperations.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 6a0864d > src/main/assemblies/sharelib.xml 07dc69c > webapp/pom.xml 4a32b54 > > > Diff: https://reviews.apache.org/r/59620/diff/7/ > > > 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). > > Andras has tested against his patch 011 with results at > https://issues.apache.org/jira/browse/OOZIE-2877?focusedCommentId=16459523&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-16459523 > > > 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. > >
