You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@oozie.apache.org by "Clay B." <cw...@clayb.net> on 2017/05/30 00:03:14 UTC
Review Request 59620: This review board request is for an action to
provide a Git action for Oozie
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59620/
-----------------------------------------------------------
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 38fb84e
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 076401d
docs/src/site/twiki/WorkflowFunctionalSpec.twiki 6655696
pom.xml ebe1d68
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/TestGitMain.java PRE-CREATION
sharelib/git/src/test/java/org/apache/oozie/action/hadoop/TestIntegrationGitActionExecutor.java PRE-CREATION
sharelib/pom.xml 190bd04
src/main/assemblies/sharelib.xml ea95c2e
webapp/pom.xml e4fdfb7
Diff: https://reviews.apache.org/r/59620/diff/1/
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.
Re: Review Request 59620: This review board request is for an action
to provide a Git action for Oozie
Posted by András Piros via Review Board <no...@reviews.apache.org>.
> On May 30, 2017, 11:08 a.m., András Piros wrote:
> > sharelib/git/src/main/java/org/apache/oozie/action/hadoop/GitMain.java
> > Lines 84-88 (patched)
> > <https://reviews.apache.org/r/59620/diff/1/?file=1734068#file1734068line84>
> >
> > Aren't there existing constants from Apache Hadoop project?
>
> Clay B. wrote:
> I am not finding a clear authoritative sources in the [https://github.com/apache/hadoop](Hadoop) project for `user.name` nor `mapred.*.job.*`; would you have a suggestion? The [JavaActionExecutor](https://github.com/apache/oozie/blob/83d4ddf45aa16649bd9fae367fa915379d5781cd/core/src/main/java/org/apache/oozie/action/hadoop/JavaActionExecutor.java#L114-L146) defines these all bespoke too sadly.
>
> I did find YARN has the resource manager in [https://github.com/apache/hadoop/blob/18c494a00c8ead768f3a868b450dceea485559df/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-api/src/main/java/org/apache/hadoop/yarn/conf/YarnConfiguration.java#L139-L148](YarnConfiguration.java).
>
> Would it make more sense to have an Oozie constants class for these? (If so, it looks like this might be a JIRA in its own scope with how often repeated some of these strings are.)
`"user.name"` -> `Hive2Credentials.USER_NAME`, `OozieClient.USER_NAME`
For the MR1 config keys there isn't a current one in the Hadoop repo, for the newer ones:
`"mapreduce.jobtracker.address"` -> [`MRConfig.MASTER_ADDRESS`](https://github.com/apache/hadoop/blob/f67237cbe7bc48a1b9088e990800b37529f1db2a/hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/java/org/apache/hadoop/mapreduce/MRConfig.java#L65)
I think less dependencies are always better, I'm for introducing these non-Oozie constants in `GitMain`, and reusing Oozie constants from code inside Oozie.
> On May 30, 2017, 11:08 a.m., András Piros wrote:
> > sharelib/git/src/main/java/org/apache/oozie/action/hadoop/GitMain.java
> > Lines 199 (patched)
> > <https://reviews.apache.org/r/59620/diff/1/?file=1734068#file1734068line199>
> >
> > Extract to another class.
>
> Clay B. wrote:
> Please pardon my lack of creativity, however, are you thinking of a repo class perhaps to encapsulate all the repository operations? Or a class for some other reason?
I'm thinking of a `GitRepositoryOperations` class to encapsulate all the repository operation, having fields like `gitSrc`, `branch`, `outputDir`, `credentialFile` initialized in constructor.
> On May 30, 2017, 11:08 a.m., András Piros wrote:
> > sharelib/git/src/main/java/org/apache/oozie/action/hadoop/GitMain.java
> > Lines 254 (patched)
> > <https://reviews.apache.org/r/59620/diff/1/?file=1734068#file1734068line254>
> >
> > Extract to another class.
>
> Clay B. wrote:
> Similarly, would this fit better in a repo class (if split off independently) or independent of the repo code and `GitMain`?
I'm thinking of a `GitRepositoryOperations` class to encapsulate all the repository operation, having fields like `gitSrc`, `branch`, `outputDir`, `credentialFile` initialized in constructor.
> On May 30, 2017, 11:08 a.m., András Piros wrote:
> > sharelib/git/src/test/java/org/apache/oozie/action/hadoop/TestIntegrationGitActionExecutor.java
> > Lines 60 (patched)
> > <https://reviews.apache.org/r/59620/diff/1/?file=1734071#file1734071line60>
> >
> > Are system props reset after this test run?
>
> Clay B. wrote:
> I am not sure what the entire chain of `ActionExecutorTestCase`'s methods are doing. However, I am not explicitly clearing this to my knowledge; do you expect issues? (For reference, it seems [`TestJavaActionExecutor()`](https://github.com/apache/oozie/blob/5998c18fde1da769e91e3ef1bcca484723730c76/core/src/test/java/org/apache/oozie/action/hadoop/TestJavaActionExecutor.java#L90) is not clearing these either?)
I'd rather not set any system properties here but go for [`Services.get().getConf().set()`](https://github.com/apache/oozie/blob/eb168360c550943828d9fe2c7bfdcd4f5e830003/core/src/test/java/org/apache/oozie/service/TestActionService.java#L52-L53), if that works.
If not, I'd still set this system property in `@Before void setUp()` and reset in `@After void tearDown()` as this one is used by other important pieces of code.
> On May 30, 2017, 11:08 a.m., András Piros wrote:
> > sharelib/git/src/test/java/org/apache/oozie/action/hadoop/TestIntegrationGitActionExecutor.java
> > Lines 80 (patched)
> > <https://reviews.apache.org/r/59620/diff/1/?file=1734071#file1734071line80>
> >
> > Please cut this into independent test cases, and name these accordingly.
>
> Clay B. wrote:
> Could you provide me more direction on how you would ike this method separated? Since the assertions in this method depend on the run state of the action under test, I would need to run the action many times (all with the same configuration); to me that would be a single case usually?
The current unit tests are more readable, closing this.
> On May 30, 2017, 11:08 a.m., András Piros wrote:
> > sharelib/git/src/test/java/org/apache/oozie/action/hadoop/TestIntegrationGitActionExecutor.java
> > Lines 153 (patched)
> > <https://reviews.apache.org/r/59620/diff/1/?file=1734071#file1734071line153>
> >
> > Extract to nested class.
>
> Clay B. wrote:
> Would you have an example in the codebase I could emulate? It looks like I am following the same pattern as:
> `./sharelib/spark/src/test/java/org/apache/oozie/action/hadoop/TestSparkActionExecutor.java`
> `./sharelib/sqoop/src/test/java/org/apache/oozie/action/hadoop/TestSqoopActionExecutor.java`
> `./sharelib/hive2/src/test/java/org/apache/oozie/action/hadoop/TestHive2ActionExecutor.java`
>
> Perhaps a separate JIRA to refactor all the test cases would be better if this would be a new pattern?
True, same pattern, sadly :) closing this one as well.
- András
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59620/#review176291
-----------------------------------------------------------
On May 4, 2018, 5:43 p.m., Clay B. wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59620/
> -----------------------------------------------------------
>
> (Updated May 4, 2018, 5:43 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/java/org/apache/oozie/cli/OozieCLI.java 4abc7502c0c9d8b59ded2baaed30c407ad073008
> 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 6f8925828090cee29a818de30a7c31db0617d816
> docs/src/site/twiki/WorkflowFunctionalSpec.twiki 76cbe21ecd79eae86885a8db77831bf2c40f2d4f
> examples/src/main/apps/git/job.properties PRE-CREATION
> examples/src/main/apps/git/workflow.xml PRE-CREATION
> pom.xml d9fe1b20f571b1a6afacb3ce53f4d409dd3d60b1
> 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 6a0864d4e5b52160dc4e1a5cb9dfc34a037b67c8
> src/main/assemblies/sharelib.xml 07dc69c8276895b254e5af0cc021cce6ebad18f4
> webapp/pom.xml 797996912b6e6381b261a69f8eb1e012fe488fdf
>
>
> Diff: https://reviews.apache.org/r/59620/diff/5/
>
>
> 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.
>
>
Re: Review Request 59620: This review board request is for an action
to provide a Git action for Oozie
Posted by András Piros via Review Board <no...@reviews.apache.org>.
> On May 30, 2017, 11:08 a.m., András Piros wrote:
> > core/src/main/java/org/apache/oozie/action/hadoop/GitActionExecutor.java
> > Lines 53 (patched)
> > <https://reviews.apache.org/r/59620/diff/1/?file=1734063#file1734063line53>
> >
> > `oozie.git.source.uri` would be better.
>
> Clay B. wrote:
> Thanks! I for some reason thought there was a pattern `oozie.<action>.<tree>` but I can not seem to find that anymore and your suggestion makes things much less redundant.
Since the action here is `git` I think it's OK this way - and it even integrates w/ the existing pattern.
- András
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59620/#review176291
-----------------------------------------------------------
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.
>
>
Re: Review Request 59620: This review board request is for an action
to provide a Git action for Oozie
Posted by "Clay B." <cw...@clayb.net>.
> On May 30, 2017, 11:08 a.m., András Piros wrote:
> > sharelib/git/src/test/java/org/apache/oozie/action/hadoop/TestIntegrationGitActionExecutor.java
> > Lines 60 (patched)
> > <https://reviews.apache.org/r/59620/diff/1/?file=1734071#file1734071line60>
> >
> > Are system props reset after this test run?
>
> Clay B. wrote:
> I am not sure what the entire chain of `ActionExecutorTestCase`'s methods are doing. However, I am not explicitly clearing this to my knowledge; do you expect issues? (For reference, it seems [`TestJavaActionExecutor()`](https://github.com/apache/oozie/blob/5998c18fde1da769e91e3ef1bcca484723730c76/core/src/test/java/org/apache/oozie/action/hadoop/TestJavaActionExecutor.java#L90) is not clearing these either?)
>
> András Piros wrote:
> I'd rather not set any system properties here but go for [`Services.get().getConf().set()`](https://github.com/apache/oozie/blob/eb168360c550943828d9fe2c7bfdcd4f5e830003/core/src/test/java/org/apache/oozie/service/TestActionService.java#L52-L53), if that works.
>
> If not, I'd still set this system property in `@Before void setUp()` and reset in `@After void tearDown()` as this one is used by other important pieces of code.
It seems this idiom is followed by the following and I am missing where they do clean-up?
* [Pig](https://github.com/apache/oozie/blob/master/sharelib/pig/src/test/java/org/apache/oozie/action/hadoop/TestPigActionExecutor.java#L81-L85)
* [Spark](https://github.com/apache/oozie/blob/299370b44e2d7b22bfe622d19f65b02c5b18282f/sharelib/spark/src/test/java/org/apache/oozie/action/hadoop/TestSparkActionExecutor.java#L66-L69)
* [MapReduce](https://github.com/apache/oozie/blob/5e1c9d362afe1b2c6423a386aeac7f04d3337f65/sharelib/streaming/src/test/java/org/apache/oozie/action/hadoop/TestMapReduceActionExecutor.java#L86-L90)
- Clay
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59620/#review176291
-----------------------------------------------------------
On June 4, 2018, 5:44 a.m., Clay B. wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59620/
> -----------------------------------------------------------
>
> (Updated June 4, 2018, 5:44 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/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 c54db34
> 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 6f35868
> 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 67526d9
>
>
> Diff: https://reviews.apache.org/r/59620/diff/6/
>
>
> 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.
>
>
Re: Review Request 59620: This review board request is for an action
to provide a Git action for Oozie
Posted by András Piros via Review Board <no...@reviews.apache.org>.
> On May 30, 2017, 11:08 a.m., András Piros wrote:
> > sharelib/git/src/test/java/org/apache/oozie/action/hadoop/TestIntegrationGitActionExecutor.java
> > Lines 60 (patched)
> > <https://reviews.apache.org/r/59620/diff/1/?file=1734071#file1734071line60>
> >
> > Are system props reset after this test run?
>
> Clay B. wrote:
> I am not sure what the entire chain of `ActionExecutorTestCase`'s methods are doing. However, I am not explicitly clearing this to my knowledge; do you expect issues? (For reference, it seems [`TestJavaActionExecutor()`](https://github.com/apache/oozie/blob/5998c18fde1da769e91e3ef1bcca484723730c76/core/src/test/java/org/apache/oozie/action/hadoop/TestJavaActionExecutor.java#L90) is not clearing these either?)
>
> András Piros wrote:
> I'd rather not set any system properties here but go for [`Services.get().getConf().set()`](https://github.com/apache/oozie/blob/eb168360c550943828d9fe2c7bfdcd4f5e830003/core/src/test/java/org/apache/oozie/service/TestActionService.java#L52-L53), if that works.
>
> If not, I'd still set this system property in `@Before void setUp()` and reset in `@After void tearDown()` as this one is used by other important pieces of code.
>
> Clay B. wrote:
> It seems this idiom is followed by the following and I am missing where they do clean-up?
>
> * [Pig](https://github.com/apache/oozie/blob/master/sharelib/pig/src/test/java/org/apache/oozie/action/hadoop/TestPigActionExecutor.java#L81-L85)
> * [Spark](https://github.com/apache/oozie/blob/299370b44e2d7b22bfe622d19f65b02c5b18282f/sharelib/spark/src/test/java/org/apache/oozie/action/hadoop/TestSparkActionExecutor.java#L66-L69)
> * [MapReduce](https://github.com/apache/oozie/blob/5e1c9d362afe1b2c6423a386aeac7f04d3337f65/sharelib/streaming/src/test/java/org/apache/oozie/action/hadoop/TestMapReduceActionExecutor.java#L86-L90)
Thanks for the explanation, dropping this issue.
- András
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59620/#review176291
-----------------------------------------------------------
On Aug. 3, 2018, 10 p.m., Clay B. wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59620/
> -----------------------------------------------------------
>
> (Updated Aug. 3, 2018, 10 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 b69d2c9
> 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
> fluent-job/fluent-job-api/pom.xml 4c9b853
> fluent-job/fluent-job-api/src/main/java/org/apache/oozie/fluentjob/api/action/GitAction.java PRE-CREATION
> fluent-job/fluent-job-api/src/main/java/org/apache/oozie/fluentjob/api/action/GitActionBuilder.java PRE-CREATION
> fluent-job/fluent-job-api/src/main/java/org/apache/oozie/fluentjob/api/mapping/ExplicitNodeConverter.java 7bb82e5
> fluent-job/fluent-job-api/src/main/java/org/apache/oozie/fluentjob/api/mapping/GitConfigurationConverter.java PRE-CREATION
> fluent-job/fluent-job-api/src/main/java/org/apache/oozie/fluentjob/api/mapping/GitPrepareConverter.java PRE-CREATION
> fluent-job/fluent-job-api/src/main/java/org/apache/oozie/fluentjob/api/serialization/WorkflowMarshaller.java ec56554
> fluent-job/fluent-job-api/src/main/resources/action_mappings.xml a5f890e
> fluent-job/fluent-job-api/src/main/xjb/bindings.xml 48f6890
> pom.xml 92358aa
> 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/GitServer.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/TestGitMain.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 fd3f89f
>
>
> Diff: https://reviews.apache.org/r/59620/diff/10/
>
>
> 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.
>
>
Re: Review Request 59620: This review board request is for an action
to provide a Git action for Oozie
Posted by "Clay B." <cw...@clayb.net>.
> On May 30, 2017, 11:08 a.m., András Piros wrote:
> >
I will keep cranking on the documentation but want to get your feedback on the changes so far; I've submitted the current code as a patch to the JIRA.
> On May 30, 2017, 11:08 a.m., András Piros wrote:
> > core/src/main/java/org/apache/oozie/action/hadoop/GitActionExecutor.java
> > Lines 53 (patched)
> > <https://reviews.apache.org/r/59620/diff/1/?file=1734063#file1734063line53>
> >
> > `oozie.git.source.uri` would be better.
Thanks! I for some reason thought there was a pattern `oozie.<action>.<tree>` but I can not seem to find that anymore and your suggestion makes things much less redundant.
> On May 30, 2017, 11:08 a.m., András Piros wrote:
> > sharelib/git/src/main/java/org/apache/oozie/action/hadoop/GitMain.java
> > Lines 82 (patched)
> > <https://reviews.apache.org/r/59620/diff/1/?file=1734068#file1734068line82>
> >
> > Why not extend `JavaMain` instead?
I did not see an advantage over LauncherMain. Is there a reason to derrive from JavaMain?
> On May 30, 2017, 11:08 a.m., András Piros wrote:
> > sharelib/git/src/main/java/org/apache/oozie/action/hadoop/GitMain.java
> > Lines 84-88 (patched)
> > <https://reviews.apache.org/r/59620/diff/1/?file=1734068#file1734068line84>
> >
> > Aren't there existing constants from Apache Hadoop project?
I am not finding a clear authoritative sources in the [https://github.com/apache/hadoop](Hadoop) project for `user.name` nor `mapred.*.job.*`; would you have a suggestion? The [JavaActionExecutor](https://github.com/apache/oozie/blob/83d4ddf45aa16649bd9fae367fa915379d5781cd/core/src/main/java/org/apache/oozie/action/hadoop/JavaActionExecutor.java#L114-L146) defines these all bespoke too sadly.
I did find YARN has the resource manager in [https://github.com/apache/hadoop/blob/18c494a00c8ead768f3a868b450dceea485559df/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-api/src/main/java/org/apache/hadoop/yarn/conf/YarnConfiguration.java#L139-L148](YarnConfiguration.java).
Would it make more sense to have an Oozie constants class for these? (If so, it looks like this might be a JIRA in its own scope with how often repeated some of these strings are.)
> On May 30, 2017, 11:08 a.m., András Piros wrote:
> > sharelib/git/src/main/java/org/apache/oozie/action/hadoop/GitMain.java
> > Lines 103 (patched)
> > <https://reviews.apache.org/r/59620/diff/1/?file=1734068#file1734068line103>
> >
> > `PROPERTIES_BLACKLIST` sounds better.
I'm happy to change this, but `DISALLOWED_PROPERTIES` came from the precident in [JavaActionExecutor](https://github.com/apache/oozie/blob/83d4ddf45aa16649bd9fae367fa915379d5781cd/core/src/main/java/org/apache/oozie/action/hadoop/JavaActionExecutor.java#L147).
> On May 30, 2017, 11:08 a.m., András Piros wrote:
> > sharelib/git/src/main/java/org/apache/oozie/action/hadoop/GitMain.java
> > Lines 144-145 (patched)
> > <https://reviews.apache.org/r/59620/diff/1/?file=1734068#file1734068line144>
> >
> > Would be goot to `LOG.error()` before re-throwing. What about throwing a more action-specific `Exception` w/ the cause caught instead?
Cool; I see how JavaMain declares its own exceptions so I'll give that a try.
> On May 30, 2017, 11:08 a.m., András Piros wrote:
> > sharelib/git/src/main/java/org/apache/oozie/action/hadoop/GitMain.java
> > Lines 154 (patched)
> > <https://reviews.apache.org/r/59620/diff/1/?file=1734068#file1734068line154>
> >
> > Is it sure that the target is always HDFS? Can't it be a local path?
Good point! Any FileSystem can work; I have updated the code to sound less opinionated as to what FileSystem one is using.
> On May 30, 2017, 11:08 a.m., András Piros wrote:
> > sharelib/git/src/main/java/org/apache/oozie/action/hadoop/GitMain.java
> > Lines 168 (patched)
> > <https://reviews.apache.org/r/59620/diff/1/?file=1734068#file1734068line168>
> >
> > `copyKeyFromHdfs()` would be a better name.
Updated to `getKeyFromFS()`
> On May 30, 2017, 11:08 a.m., András Piros wrote:
> > sharelib/git/src/main/java/org/apache/oozie/action/hadoop/GitMain.java
> > Lines 199 (patched)
> > <https://reviews.apache.org/r/59620/diff/1/?file=1734068#file1734068line199>
> >
> > Extract to another class.
Please pardon my lack of creativity, however, are you thinking of a repo class perhaps to encapsulate all the repository operations? Or a class for some other reason?
> On May 30, 2017, 11:08 a.m., András Piros wrote:
> > sharelib/git/src/main/java/org/apache/oozie/action/hadoop/GitMain.java
> > Lines 254 (patched)
> > <https://reviews.apache.org/r/59620/diff/1/?file=1734068#file1734068line254>
> >
> > Extract to another class.
Similarly, would this fit better in a repo class (if split off independently) or independent of the repo code and `GitMain`?
> On May 30, 2017, 11:08 a.m., András Piros wrote:
> > sharelib/git/src/test/java/org/apache/oozie/action/hadoop/TestGitMain.java
> > Lines 125-128 (patched)
> > <https://reviews.apache.org/r/59620/diff/1/?file=1734070#file1734070line125>
> >
> > Extract method.
Moved this to a `setUp()` method.
> On May 30, 2017, 11:08 a.m., András Piros wrote:
> > sharelib/git/src/test/java/org/apache/oozie/action/hadoop/TestIntegrationGitActionExecutor.java
> > Lines 60 (patched)
> > <https://reviews.apache.org/r/59620/diff/1/?file=1734071#file1734071line60>
> >
> > Are system props reset after this test run?
I am not sure what the entire chain of `ActionExecutorTestCase`'s methods are doing. However, I am not explicitly clearing this to my knowledge; do you expect issues? (For reference, it seems [`TestJavaActionExecutor()`](https://github.com/apache/oozie/blob/5998c18fde1da769e91e3ef1bcca484723730c76/core/src/test/java/org/apache/oozie/action/hadoop/TestJavaActionExecutor.java#L90) is not clearing these either?)
> On May 30, 2017, 11:08 a.m., András Piros wrote:
> > sharelib/git/src/test/java/org/apache/oozie/action/hadoop/TestIntegrationGitActionExecutor.java
> > Lines 80 (patched)
> > <https://reviews.apache.org/r/59620/diff/1/?file=1734071#file1734071line80>
> >
> > Please cut this into independent test cases, and name these accordingly.
Could you provide me more direction on how you would ike this method separated? Since the assertions in this method depend on the run state of the action under test, I would need to run the action many times (all with the same configuration); to me that would be a single case usually?
> On May 30, 2017, 11:08 a.m., András Piros wrote:
> > sharelib/git/src/test/java/org/apache/oozie/action/hadoop/TestIntegrationGitActionExecutor.java
> > Lines 153 (patched)
> > <https://reviews.apache.org/r/59620/diff/1/?file=1734071#file1734071line153>
> >
> > Extract to nested class.
Would you have an example in the codebase I could emulate? It looks like I am following the same pattern as:
`./sharelib/spark/src/test/java/org/apache/oozie/action/hadoop/TestSparkActionExecutor.java`
`./sharelib/sqoop/src/test/java/org/apache/oozie/action/hadoop/TestSqoopActionExecutor.java`
`./sharelib/hive2/src/test/java/org/apache/oozie/action/hadoop/TestHive2ActionExecutor.java`
Perhaps a separate JIRA to refactor all the test cases would be better if this would be a new pattern?
- Clay
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59620/#review176291
-----------------------------------------------------------
On May 30, 2017, 12:03 a.m., Clay B. wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59620/
> -----------------------------------------------------------
>
> (Updated May 30, 2017, 12:03 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 38fb84e
> 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 076401d
> docs/src/site/twiki/WorkflowFunctionalSpec.twiki 6655696
> pom.xml ebe1d68
> 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/TestGitMain.java PRE-CREATION
> sharelib/git/src/test/java/org/apache/oozie/action/hadoop/TestIntegrationGitActionExecutor.java PRE-CREATION
> sharelib/pom.xml 190bd04
> src/main/assemblies/sharelib.xml ea95c2e
> webapp/pom.xml e4fdfb7
>
>
> Diff: https://reviews.apache.org/r/59620/diff/1/
>
>
> 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.
>
>
Re: Review Request 59620: This review board request is for an action
to provide a Git action for Oozie
Posted by "Clay B." <cw...@clayb.net>.
> On May 30, 2017, 11:08 a.m., András Piros wrote:
> > sharelib/git/src/main/java/org/apache/oozie/action/hadoop/GitMain.java
> > Lines 84-88 (patched)
> > <https://reviews.apache.org/r/59620/diff/1/?file=1734068#file1734068line84>
> >
> > Aren't there existing constants from Apache Hadoop project?
>
> Clay B. wrote:
> I am not finding a clear authoritative sources in the [https://github.com/apache/hadoop](Hadoop) project for `user.name` nor `mapred.*.job.*`; would you have a suggestion? The [JavaActionExecutor](https://github.com/apache/oozie/blob/83d4ddf45aa16649bd9fae367fa915379d5781cd/core/src/main/java/org/apache/oozie/action/hadoop/JavaActionExecutor.java#L114-L146) defines these all bespoke too sadly.
>
> I did find YARN has the resource manager in [https://github.com/apache/hadoop/blob/18c494a00c8ead768f3a868b450dceea485559df/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-api/src/main/java/org/apache/hadoop/yarn/conf/YarnConfiguration.java#L139-L148](YarnConfiguration.java).
>
> Would it make more sense to have an Oozie constants class for these? (If so, it looks like this might be a JIRA in its own scope with how often repeated some of these strings are.)
>
> András Piros wrote:
> `"user.name"` -> `Hive2Credentials.USER_NAME`, `OozieClient.USER_NAME`
>
> For the MR1 config keys there isn't a current one in the Hadoop repo, for the newer ones:
> `"mapreduce.jobtracker.address"` -> [`MRConfig.MASTER_ADDRESS`](https://github.com/apache/hadoop/blob/f67237cbe7bc48a1b9088e990800b37529f1db2a/hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/java/org/apache/hadoop/mapreduce/MRConfig.java#L65)
>
> I think less dependencies are always better, I'm for introducing these non-Oozie constants in `GitMain`, and reusing Oozie constants from code inside Oozie.
I think these are all gone in the latest patch from you Andras?
- Clay
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59620/#review176291
-----------------------------------------------------------
On May 4, 2018, 5:43 p.m., Clay B. wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59620/
> -----------------------------------------------------------
>
> (Updated May 4, 2018, 5:43 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/java/org/apache/oozie/cli/OozieCLI.java 4abc7502c0c9d8b59ded2baaed30c407ad073008
> 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 6f8925828090cee29a818de30a7c31db0617d816
> docs/src/site/twiki/WorkflowFunctionalSpec.twiki 76cbe21ecd79eae86885a8db77831bf2c40f2d4f
> examples/src/main/apps/git/job.properties PRE-CREATION
> examples/src/main/apps/git/workflow.xml PRE-CREATION
> pom.xml d9fe1b20f571b1a6afacb3ce53f4d409dd3d60b1
> 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 6a0864d4e5b52160dc4e1a5cb9dfc34a037b67c8
> src/main/assemblies/sharelib.xml 07dc69c8276895b254e5af0cc021cce6ebad18f4
> webapp/pom.xml 797996912b6e6381b261a69f8eb1e012fe488fdf
>
>
> Diff: https://reviews.apache.org/r/59620/diff/5/
>
>
> 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.
>
>
Re: Review Request 59620: This review board request is for an action
to provide a Git action for Oozie
Posted by András Piros <an...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59620/#review176291
-----------------------------------------------------------
core/src/main/java/org/apache/oozie/action/hadoop/GitActionExecutor.java
Lines 53 (patched)
<https://reviews.apache.org/r/59620/#comment249652>
`oozie.git.source.uri` would be better.
core/src/main/java/org/apache/oozie/action/hadoop/GitActionExecutor.java
Lines 54 (patched)
<https://reviews.apache.org/r/59620/#comment249653>
`oozie.git.branch` would be better.
core/src/main/java/org/apache/oozie/action/hadoop/GitActionExecutor.java
Lines 73 (patched)
<https://reviews.apache.org/r/59620/#comment249654>
What about throwing an `ActionExecutorException` instead?
core/src/main/java/org/apache/oozie/action/hadoop/GitActionExecutor.java
Lines 110-114 (patched)
<https://reviews.apache.org/r/59620/#comment249655>
Extract method.
core/src/main/java/org/apache/oozie/action/hadoop/GitActionExecutor.java
Lines 116-120 (patched)
<https://reviews.apache.org/r/59620/#comment249656>
Extract method.
core/src/main/java/org/apache/oozie/action/hadoop/GitActionExecutor.java
Lines 122-126 (patched)
<https://reviews.apache.org/r/59620/#comment249657>
Extract method.
core/src/main/java/org/apache/oozie/action/hadoop/GitActionExecutor.java
Lines 130-134 (patched)
<https://reviews.apache.org/r/59620/#comment249658>
Extract method.
core/src/main/java/org/apache/oozie/action/hadoop/GitActionExecutor.java
Lines 138-142 (patched)
<https://reviews.apache.org/r/59620/#comment249659>
Extract method.
core/src/main/java/org/apache/oozie/action/hadoop/GitActionExecutor.java
Lines 154-158 (patched)
<https://reviews.apache.org/r/59620/#comment249660>
Extract method.
core/src/main/java/org/apache/oozie/action/hadoop/GitActionExecutor.java
Lines 161-164 (patched)
<https://reviews.apache.org/r/59620/#comment249661>
Extract method.
core/src/main/java/org/apache/oozie/action/hadoop/GitActionExecutor.java
Lines 166-169 (patched)
<https://reviews.apache.org/r/59620/#comment249662>
Extract method.
docs/src/site/twiki/WorkflowFunctionalSpec.twiki
Lines 1617 (patched)
<https://reviews.apache.org/r/59620/#comment249663>
Please resolve this TODO.
docs/src/site/twiki/WorkflowFunctionalSpec.twiki
Lines 1621 (patched)
<https://reviews.apache.org/r/59620/#comment249664>
Please resolve this TODO.
docs/src/site/twiki/WorkflowFunctionalSpec.twiki
Lines 1624 (patched)
<https://reviews.apache.org/r/59620/#comment249665>
Please resolve this TODO.
sharelib/git/src/main/java/org/apache/oozie/action/hadoop/GitMain.java
Lines 82 (patched)
<https://reviews.apache.org/r/59620/#comment249666>
Why not extend `JavaMain` instead?
sharelib/git/src/main/java/org/apache/oozie/action/hadoop/GitMain.java
Lines 84-88 (patched)
<https://reviews.apache.org/r/59620/#comment249667>
Aren't there existing constants from Apache Hadoop project?
sharelib/git/src/main/java/org/apache/oozie/action/hadoop/GitMain.java
Lines 103 (patched)
<https://reviews.apache.org/r/59620/#comment249668>
`PROPERTIES_BLACKLIST` sounds better.
sharelib/git/src/main/java/org/apache/oozie/action/hadoop/GitMain.java
Lines 129 (patched)
<https://reviews.apache.org/r/59620/#comment249669>
Sounds like a good idea extracting `oozie.action.conf.xml` to a constant.
sharelib/git/src/main/java/org/apache/oozie/action/hadoop/GitMain.java
Lines 144-145 (patched)
<https://reviews.apache.org/r/59620/#comment249670>
Would be goot to `LOG.error()` before re-throwing. What about throwing a more action-specific `Exception` w/ the cause caught instead?
sharelib/git/src/main/java/org/apache/oozie/action/hadoop/GitMain.java
Lines 154 (patched)
<https://reviews.apache.org/r/59620/#comment249672>
Is it sure that the target is always HDFS? Can't it be a local path?
sharelib/git/src/main/java/org/apache/oozie/action/hadoop/GitMain.java
Lines 156-157 (patched)
<https://reviews.apache.org/r/59620/#comment249671>
Would be goot to `LOG.error()` before re-throwing. What about throwing a more action-specific `Exception` w/ the cause caught instead?
sharelib/git/src/main/java/org/apache/oozie/action/hadoop/GitMain.java
Lines 168 (patched)
<https://reviews.apache.org/r/59620/#comment249675>
`copyKeyFromHdfs()` would be a better name.
sharelib/git/src/main/java/org/apache/oozie/action/hadoop/GitMain.java
Lines 179-182 (patched)
<https://reviews.apache.org/r/59620/#comment249676>
Better to extract a `String` variable and reuse.
sharelib/git/src/main/java/org/apache/oozie/action/hadoop/GitMain.java
Lines 186-187 (patched)
<https://reviews.apache.org/r/59620/#comment249677>
Better to extract a `String` variable and reuse.
sharelib/git/src/main/java/org/apache/oozie/action/hadoop/GitMain.java
Lines 199 (patched)
<https://reviews.apache.org/r/59620/#comment249678>
Extract to another class.
sharelib/git/src/main/java/org/apache/oozie/action/hadoop/GitMain.java
Lines 254 (patched)
<https://reviews.apache.org/r/59620/#comment249679>
Extract to another class.
sharelib/git/src/main/java/org/apache/oozie/action/hadoop/GitMain.java
Lines 299-303 (patched)
<https://reviews.apache.org/r/59620/#comment249680>
Extract method.
sharelib/git/src/main/java/org/apache/oozie/action/hadoop/GitMain.java
Lines 305-309 (patched)
<https://reviews.apache.org/r/59620/#comment249681>
Extract method.
sharelib/git/src/main/java/org/apache/oozie/action/hadoop/GitMain.java
Lines 311-315 (patched)
<https://reviews.apache.org/r/59620/#comment249682>
Extract method.
sharelib/git/src/main/java/org/apache/oozie/action/hadoop/GitMain.java
Lines 317-321 (patched)
<https://reviews.apache.org/r/59620/#comment249683>
Extract method.
sharelib/git/src/main/java/org/apache/oozie/action/hadoop/GitMain.java
Lines 323-327 (patched)
<https://reviews.apache.org/r/59620/#comment249684>
Extract method.
sharelib/git/src/main/java/org/apache/oozie/action/hadoop/GitMain.java
Lines 329-333 (patched)
<https://reviews.apache.org/r/59620/#comment249685>
Extract method.
sharelib/git/src/main/java/org/apache/oozie/action/hadoop/GitMain.java
Lines 334-345 (patched)
<https://reviews.apache.org/r/59620/#comment249687>
Extract method.
sharelib/git/src/main/java/org/apache/oozie/action/hadoop/GitMain.java
Lines 347-351 (patched)
<https://reviews.apache.org/r/59620/#comment249686>
Extract method.
sharelib/git/src/main/java/org/apache/oozie/action/hadoop/GitMain.java
Lines 352-361 (patched)
<https://reviews.apache.org/r/59620/#comment249688>
Extract method.
sharelib/git/src/main/java/org/apache/oozie/action/hadoop/GitMain.java
Lines 370-374 (patched)
<https://reviews.apache.org/r/59620/#comment249689>
Extract method.
sharelib/git/src/main/java/org/apache/oozie/action/hadoop/GitMain.java
Lines 376-380 (patched)
<https://reviews.apache.org/r/59620/#comment249690>
Extract method.
sharelib/git/src/test/java/org/apache/oozie/action/hadoop/TestGitMain.java
Lines 102-114 (patched)
<https://reviews.apache.org/r/59620/#comment249691>
Pls. remove dead code.
sharelib/git/src/test/java/org/apache/oozie/action/hadoop/TestGitMain.java
Lines 117-121 (patched)
<https://reviews.apache.org/r/59620/#comment249692>
Pls. remove dead code.
sharelib/git/src/test/java/org/apache/oozie/action/hadoop/TestGitMain.java
Lines 125-128 (patched)
<https://reviews.apache.org/r/59620/#comment249695>
Extract method.
sharelib/git/src/test/java/org/apache/oozie/action/hadoop/TestGitMain.java
Lines 131-133 (patched)
<https://reviews.apache.org/r/59620/#comment249696>
Please employ that `GitMain.nameNode` is (package-)protected instead, and use `@VisibleForTesting`.
sharelib/git/src/test/java/org/apache/oozie/action/hadoop/TestGitMain.java
Lines 136 (patched)
<https://reviews.apache.org/r/59620/#comment249693>
Pls. remove dead code.
sharelib/git/src/test/java/org/apache/oozie/action/hadoop/TestGitMain.java
Lines 138-144 (patched)
<https://reviews.apache.org/r/59620/#comment249697>
Use try-with-resources instead.
sharelib/git/src/test/java/org/apache/oozie/action/hadoop/TestGitMain.java
Lines 146-187 (patched)
<https://reviews.apache.org/r/59620/#comment249694>
Pls. remove dead code.
sharelib/git/src/test/java/org/apache/oozie/action/hadoop/TestIntegrationGitActionExecutor.java
Lines 60 (patched)
<https://reviews.apache.org/r/59620/#comment249698>
Are system props reset after this test run?
sharelib/git/src/test/java/org/apache/oozie/action/hadoop/TestIntegrationGitActionExecutor.java
Lines 80 (patched)
<https://reviews.apache.org/r/59620/#comment249699>
Please cut this into independent test cases, and name these accordingly.
sharelib/git/src/test/java/org/apache/oozie/action/hadoop/TestIntegrationGitActionExecutor.java
Lines 153 (patched)
<https://reviews.apache.org/r/59620/#comment249700>
Extract to nested class.
- András Piros
On May 30, 2017, 12:03 a.m., Clay B. wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59620/
> -----------------------------------------------------------
>
> (Updated May 30, 2017, 12:03 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 38fb84e
> 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 076401d
> docs/src/site/twiki/WorkflowFunctionalSpec.twiki 6655696
> pom.xml ebe1d68
> 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/TestGitMain.java PRE-CREATION
> sharelib/git/src/test/java/org/apache/oozie/action/hadoop/TestIntegrationGitActionExecutor.java PRE-CREATION
> sharelib/pom.xml 190bd04
> src/main/assemblies/sharelib.xml ea95c2e
> webapp/pom.xml e4fdfb7
>
>
> Diff: https://reviews.apache.org/r/59620/diff/1/
>
>
> 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.
>
>
Re: Review Request 59620: This review board request is for an action
to provide a Git action for Oozie
Posted by András Piros via Review Board <no...@reviews.apache.org>.
> On June 5, 2018, 4:46 p.m., Robert Kanter wrote:
> > sharelib/git/src/main/java/org/apache/oozie/action/hadoop/GitMain.java
> > Lines 216 (patched)
> > <https://reviews.apache.org/r/59620/diff/6/?file=2034895#file2034895line216>
> >
> > Is this necessary? Why not simply throw whatever exception (IOException, IllegalArgumentException, etc)? We don't seem to be doing any special handling or anything for GitMainExceptions.
>
> Clay B. wrote:
> I was simply copying the idiom from JavaMain but do not have any broader plan for the exception; the same is true for `GitOperationsException`.
>
> Robert Kanter wrote:
> I forget exactly the reason off the top of my head, but we needed to wrap Exceptions in another layer in the Java Action to handle a certain case. I believe it was related to the fact that we'd be running unknown Java code (provided by the user), but I'm not sure. If you look at LauncherAM, it's actually doing something with JavaMainException. Anyway, you can see that none of the other actions do this, so I don't think we need a GitMainException.
Removed `GitMainException`.
- András
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59620/#review204323
-----------------------------------------------------------
On Aug. 3, 2018, 10 p.m., Clay B. wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59620/
> -----------------------------------------------------------
>
> (Updated Aug. 3, 2018, 10 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 b69d2c9
> 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
> fluent-job/fluent-job-api/pom.xml 4c9b853
> fluent-job/fluent-job-api/src/main/java/org/apache/oozie/fluentjob/api/action/GitAction.java PRE-CREATION
> fluent-job/fluent-job-api/src/main/java/org/apache/oozie/fluentjob/api/action/GitActionBuilder.java PRE-CREATION
> fluent-job/fluent-job-api/src/main/java/org/apache/oozie/fluentjob/api/mapping/ExplicitNodeConverter.java 7bb82e5
> fluent-job/fluent-job-api/src/main/java/org/apache/oozie/fluentjob/api/mapping/GitConfigurationConverter.java PRE-CREATION
> fluent-job/fluent-job-api/src/main/java/org/apache/oozie/fluentjob/api/mapping/GitPrepareConverter.java PRE-CREATION
> fluent-job/fluent-job-api/src/main/java/org/apache/oozie/fluentjob/api/serialization/WorkflowMarshaller.java ec56554
> fluent-job/fluent-job-api/src/main/resources/action_mappings.xml a5f890e
> fluent-job/fluent-job-api/src/main/xjb/bindings.xml 48f6890
> pom.xml 92358aa
> 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/GitServer.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/TestGitMain.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 fd3f89f
>
>
> Diff: https://reviews.apache.org/r/59620/diff/10/
>
>
> 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.
>
>
Re: Review Request 59620: This review board request is for an action
to provide a Git action for Oozie
Posted by Robert Kanter via Review Board <no...@reviews.apache.org>.
> On June 5, 2018, 4:46 p.m., Robert Kanter wrote:
> > sharelib/git/src/main/java/org/apache/oozie/action/hadoop/GitMain.java
> > Lines 216 (patched)
> > <https://reviews.apache.org/r/59620/diff/6/?file=2034895#file2034895line216>
> >
> > Is this necessary? Why not simply throw whatever exception (IOException, IllegalArgumentException, etc)? We don't seem to be doing any special handling or anything for GitMainExceptions.
>
> Clay B. wrote:
> I was simply copying the idiom from JavaMain but do not have any broader plan for the exception; the same is true for `GitOperationsException`.
I forget exactly the reason off the top of my head, but we needed to wrap Exceptions in another layer in the Java Action to handle a certain case. I believe it was related to the fact that we'd be running unknown Java code (provided by the user), but I'm not sure. If you look at LauncherAM, it's actually doing something with JavaMainException. Anyway, you can see that none of the other actions do this, so I don't think we need a GitMainException.
- Robert
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59620/#review204323
-----------------------------------------------------------
On June 4, 2018, 5:44 a.m., Clay B. wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59620/
> -----------------------------------------------------------
>
> (Updated June 4, 2018, 5:44 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/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 c54db34
> 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 6f35868
> 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 67526d9
>
>
> Diff: https://reviews.apache.org/r/59620/diff/6/
>
>
> 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.
>
>
Re: Review Request 59620: This review board request is for an action
to provide a Git action for Oozie
Posted by "Clay B." <cw...@clayb.net>.
> On June 5, 2018, 4:46 p.m., Robert Kanter wrote:
> > sharelib/git/src/test/java/org/apache/oozie/action/hadoop/TestGitActionExecutor.java
> > Lines 36 (patched)
> > <https://reviews.apache.org/r/59620/diff/6/?file=2034897#file2034897line36>
> >
> > As Peter Bacsko pointed out on the JIRA, we shouldn't be fetching a remote repo during a unit test. Perhaps JGit has some easy way of mocking this or doing it locally?
Corrected to be a full unit-test spawning its own Git protocol server now.
- Clay
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59620/#review204323
-----------------------------------------------------------
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.
>
>
Re: Review Request 59620: This review board request is for an action
to provide a Git action for Oozie
Posted by "Clay B." <cw...@clayb.net>.
> On June 5, 2018, 4:46 p.m., Robert Kanter wrote:
> > sharelib/git/src/main/java/org/apache/oozie/action/hadoop/GitMain.java
> > Lines 216 (patched)
> > <https://reviews.apache.org/r/59620/diff/6/?file=2034895#file2034895line216>
> >
> > Is this necessary? Why not simply throw whatever exception (IOException, IllegalArgumentException, etc)? We don't seem to be doing any special handling or anything for GitMainExceptions.
I was simply copying the idiom from JavaMain but do not have any broader plan for the exception; the same is true for `GitOperationsException`.
- Clay
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59620/#review204323
-----------------------------------------------------------
On June 4, 2018, 5:44 a.m., Clay B. wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59620/
> -----------------------------------------------------------
>
> (Updated June 4, 2018, 5:44 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/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 c54db34
> 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 6f35868
> 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 67526d9
>
>
> Diff: https://reviews.apache.org/r/59620/diff/6/
>
>
> 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.
>
>
Re: Review Request 59620: This review board request is for an action
to provide a Git action for Oozie
Posted by Robert Kanter via Review Board <no...@reviews.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59620/#review204323
-----------------------------------------------------------
examples/src/main/apps/git/job.properties
Lines 9 (patched)
<https://reviews.apache.org/r/59620/#comment286760>
trailing whitespace; there's some other cases here and in other files too
sharelib/git/pom.xml
Lines 107-118 (patched)
<https://reviews.apache.org/r/59620/#comment286761>
This can be removed (It was needed some time ago in each sharelib pom file, but not anymore).
sharelib/git/src/main/java/org/apache/oozie/action/hadoop/GitMain.java
Lines 216 (patched)
<https://reviews.apache.org/r/59620/#comment286762>
Is this necessary? Why not simply throw whatever exception (IOException, IllegalArgumentException, etc)? We don't seem to be doing any special handling or anything for GitMainExceptions.
sharelib/git/src/test/java/org/apache/oozie/action/hadoop/TestGitActionExecutor.java
Lines 33 (patched)
<https://reviews.apache.org/r/59620/#comment286763>
"is" --> "in"
sharelib/git/src/test/java/org/apache/oozie/action/hadoop/TestGitActionExecutor.java
Lines 36 (patched)
<https://reviews.apache.org/r/59620/#comment286764>
As Peter Bacsko pointed out on the JIRA, we shouldn't be fetching a remote repo during a unit test. Perhaps JGit has some easy way of mocking this or doing it locally?
- Robert Kanter
On June 4, 2018, 5:44 a.m., Clay B. wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59620/
> -----------------------------------------------------------
>
> (Updated June 4, 2018, 5:44 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/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 c54db34
> 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 6f35868
> 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 67526d9
>
>
> Diff: https://reviews.apache.org/r/59620/diff/6/
>
>
> 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.
>
>
Re: Review Request 59620: This review board request is for an action
to provide a Git action for Oozie
Posted by András Piros via Review Board <no...@reviews.apache.org>.
> On Aug. 7, 2018, 12:38 p.m., Peter Bacsko wrote:
> > core/src/main/java/org/apache/oozie/action/hadoop/GitActionExecutor.java
> > Lines 182-183 (patched)
> > <https://reviews.apache.org/r/59620/diff/10/?file=2067411#file2067411line182>
> >
> > These nested ifs can be merged.
Done.
> On Aug. 7, 2018, 12:38 p.m., Peter Bacsko wrote:
> > sharelib/git/src/main/java/org/apache/oozie/action/hadoop/GitMain.java
> > Lines 200-201 (patched)
> > <https://reviews.apache.org/r/59620/diff/10/?file=2067428#file2067428line200>
> >
> > Do we need this?
Removed.
> On Aug. 7, 2018, 12:38 p.m., Peter Bacsko wrote:
> > sharelib/git/src/main/java/org/apache/oozie/action/hadoop/GitOperations.java
> > Lines 92 (patched)
> > <https://reviews.apache.org/r/59620/diff/10/?file=2067429#file2067429line92>
> >
> > Typo: "Checkiging"
Fixed.
> On Aug. 7, 2018, 12:38 p.m., Peter Bacsko wrote:
> > sharelib/git/src/test/java/org/apache/oozie/action/hadoop/TestGitActionExecutor.java
> > Lines 114 (patched)
> > <https://reviews.apache.org/r/59620/diff/10/?file=2067431#file2067431line114>
> >
> > Better error message would be "Expected ActionExecutorException"
Done.
> On Aug. 7, 2018, 12:38 p.m., Peter Bacsko wrote:
> > sharelib/git/src/test/java/org/apache/oozie/action/hadoop/TestGitActionExecutor.java
> > Lines 117 (patched)
> > <https://reviews.apache.org/r/59620/diff/10/?file=2067431#file2067431line117>
> >
> > Better error message would be "Unexpected exception message" or sth like that
Done.
> On Aug. 7, 2018, 12:38 p.m., Peter Bacsko wrote:
> > sharelib/git/src/test/java/org/apache/oozie/action/hadoop/TestGitActionExecutor.java
> > Lines 148-154 (patched)
> > <https://reviews.apache.org/r/59620/diff/10/?file=2067431#file2067431line148>
> >
> > What are we testing here exactly?
> >
> > I can't see any assert() calls.
Done.
> On Aug. 7, 2018, 12:38 p.m., Peter Bacsko wrote:
> > sharelib/git/src/test/java/org/apache/oozie/action/hadoop/TestGitActionExecutor.java
> > Lines 183-188 (patched)
> > <https://reviews.apache.org/r/59620/diff/10/?file=2067431#file2067431line183>
> >
> > Same here
Done.
> On Aug. 7, 2018, 12:38 p.m., Peter Bacsko wrote:
> > sharelib/git/src/test/java/org/apache/oozie/action/hadoop/TestGitMain.java
> > Lines 101-104 (patched)
> > <https://reviews.apache.org/r/59620/diff/10/?file=2067432#file2067432line101>
> >
> > This should be replaced with a waitFor() method call - see Oozie codebase for examples.
Done.
> On Aug. 7, 2018, 12:38 p.m., Peter Bacsko wrote:
> > sharelib/git/src/test/java/org/apache/oozie/action/hadoop/TestIntegrationGitActionExecutor.java
> > Lines 30 (patched)
> > <https://reviews.apache.org/r/59620/diff/10/?file=2067433#file2067433line30>
> >
> > Unused import
Done.
> On Aug. 7, 2018, 12:38 p.m., Peter Bacsko wrote:
> > sharelib/git/src/test/java/org/apache/oozie/action/hadoop/TestIntegrationGitActionExecutor.java
> > Lines 86 (patched)
> > <https://reviews.apache.org/r/59620/diff/10/?file=2067433#file2067433line86>
> >
> > readContent variable is unused
Done.
- András
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59620/#review206939
-----------------------------------------------------------
On Aug. 3, 2018, 10 p.m., Clay B. wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59620/
> -----------------------------------------------------------
>
> (Updated Aug. 3, 2018, 10 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 b69d2c9
> 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
> fluent-job/fluent-job-api/pom.xml 4c9b853
> fluent-job/fluent-job-api/src/main/java/org/apache/oozie/fluentjob/api/action/GitAction.java PRE-CREATION
> fluent-job/fluent-job-api/src/main/java/org/apache/oozie/fluentjob/api/action/GitActionBuilder.java PRE-CREATION
> fluent-job/fluent-job-api/src/main/java/org/apache/oozie/fluentjob/api/mapping/ExplicitNodeConverter.java 7bb82e5
> fluent-job/fluent-job-api/src/main/java/org/apache/oozie/fluentjob/api/mapping/GitConfigurationConverter.java PRE-CREATION
> fluent-job/fluent-job-api/src/main/java/org/apache/oozie/fluentjob/api/mapping/GitPrepareConverter.java PRE-CREATION
> fluent-job/fluent-job-api/src/main/java/org/apache/oozie/fluentjob/api/serialization/WorkflowMarshaller.java ec56554
> fluent-job/fluent-job-api/src/main/resources/action_mappings.xml a5f890e
> fluent-job/fluent-job-api/src/main/xjb/bindings.xml 48f6890
> pom.xml 92358aa
> 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/GitServer.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/TestGitMain.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 fd3f89f
>
>
> Diff: https://reviews.apache.org/r/59620/diff/10/
>
>
> 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.
>
>
Re: Review Request 59620: This review board request is for an action
to provide a Git action for Oozie
Posted by Peter Bacsko via Review Board <no...@reviews.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59620/#review206939
-----------------------------------------------------------
core/src/main/java/org/apache/oozie/action/hadoop/GitActionExecutor.java
Lines 182-183 (patched)
<https://reviews.apache.org/r/59620/#comment290106>
These nested ifs can be merged.
sharelib/git/src/main/java/org/apache/oozie/action/hadoop/GitMain.java
Lines 200-201 (patched)
<https://reviews.apache.org/r/59620/#comment290107>
Do we need this?
sharelib/git/src/main/java/org/apache/oozie/action/hadoop/GitOperations.java
Lines 92 (patched)
<https://reviews.apache.org/r/59620/#comment290108>
Typo: "Checkiging"
sharelib/git/src/test/java/org/apache/oozie/action/hadoop/GitServer.java
Lines 52 (patched)
<https://reviews.apache.org/r/59620/#comment290116>
This solution is much better than the original.
Just one more thing: perhaps instead of hard-coding the port 9418, we could pick one dynamically. Or at least check if it's not occupied by some other process.
sharelib/git/src/test/java/org/apache/oozie/action/hadoop/TestGitActionExecutor.java
Lines 57 (patched)
<https://reviews.apache.org/r/59620/#comment290117>
I'm just curious - how will Git know that it has to connect to the simple local server instead of Github?
sharelib/git/src/test/java/org/apache/oozie/action/hadoop/TestGitActionExecutor.java
Lines 114 (patched)
<https://reviews.apache.org/r/59620/#comment290114>
Better error message would be "Expected ActionExecutorException"
sharelib/git/src/test/java/org/apache/oozie/action/hadoop/TestGitActionExecutor.java
Lines 117 (patched)
<https://reviews.apache.org/r/59620/#comment290115>
Better error message would be "Unexpected exception message" or sth like that
sharelib/git/src/test/java/org/apache/oozie/action/hadoop/TestGitActionExecutor.java
Lines 148-154 (patched)
<https://reviews.apache.org/r/59620/#comment290109>
What are we testing here exactly?
I can't see any assert() calls.
sharelib/git/src/test/java/org/apache/oozie/action/hadoop/TestGitActionExecutor.java
Lines 183-188 (patched)
<https://reviews.apache.org/r/59620/#comment290110>
Same here
sharelib/git/src/test/java/org/apache/oozie/action/hadoop/TestGitMain.java
Lines 101-104 (patched)
<https://reviews.apache.org/r/59620/#comment290113>
This should be replaced with a waitFor() method call - see Oozie codebase for examples.
sharelib/git/src/test/java/org/apache/oozie/action/hadoop/TestIntegrationGitActionExecutor.java
Lines 30 (patched)
<https://reviews.apache.org/r/59620/#comment290111>
Unused import
sharelib/git/src/test/java/org/apache/oozie/action/hadoop/TestIntegrationGitActionExecutor.java
Lines 86 (patched)
<https://reviews.apache.org/r/59620/#comment290112>
readContent variable is unused
- Peter Bacsko
On aug. 3, 2018, 10 du, Clay B. wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59620/
> -----------------------------------------------------------
>
> (Updated aug. 3, 2018, 10 du)
>
>
> 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 b69d2c9
> 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
> fluent-job/fluent-job-api/pom.xml 4c9b853
> fluent-job/fluent-job-api/src/main/java/org/apache/oozie/fluentjob/api/action/GitAction.java PRE-CREATION
> fluent-job/fluent-job-api/src/main/java/org/apache/oozie/fluentjob/api/action/GitActionBuilder.java PRE-CREATION
> fluent-job/fluent-job-api/src/main/java/org/apache/oozie/fluentjob/api/mapping/ExplicitNodeConverter.java 7bb82e5
> fluent-job/fluent-job-api/src/main/java/org/apache/oozie/fluentjob/api/mapping/GitConfigurationConverter.java PRE-CREATION
> fluent-job/fluent-job-api/src/main/java/org/apache/oozie/fluentjob/api/mapping/GitPrepareConverter.java PRE-CREATION
> fluent-job/fluent-job-api/src/main/java/org/apache/oozie/fluentjob/api/serialization/WorkflowMarshaller.java ec56554
> fluent-job/fluent-job-api/src/main/resources/action_mappings.xml a5f890e
> fluent-job/fluent-job-api/src/main/xjb/bindings.xml 48f6890
> pom.xml 92358aa
> 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/GitServer.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/TestGitMain.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 fd3f89f
>
>
> Diff: https://reviews.apache.org/r/59620/diff/10/
>
>
> 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.
>
>
Re: Review Request 59620: This review board request is for an action
to provide a Git action for Oozie
Posted by András Piros via Review Board <no...@reviews.apache.org>.
> On Aug. 8, 2018, 9:57 a.m., Peter Bacsko wrote:
> > core/src/main/java/org/apache/oozie/action/hadoop/GitActionExecutor.java
> > Lines 152 (patched)
> > <https://reviews.apache.org/r/59620/diff/10/?file=2067411#file2067411line152>
> >
> > Better naming suggested: ActionConfVerifier
Done.
> On Aug. 8, 2018, 9:57 a.m., Peter Bacsko wrote:
> > sharelib/git/src/main/java/org/apache/oozie/action/hadoop/GitMain.java
> > Lines 38 (patched)
> > <https://reviews.apache.org/r/59620/diff/10/?file=2067428#file2067428line38>
> >
> > General thoughts: this class has quite a few protected methods. We have to think about whether it's necessary or not. If the class is not going to be subclassed, making them "private" is preferable.
Done.
> On Aug. 8, 2018, 9:57 a.m., Peter Bacsko wrote:
> > sharelib/git/src/main/java/org/apache/oozie/action/hadoop/GitMain.java
> > Lines 43 (patched)
> > <https://reviews.apache.org/r/59620/diff/10/?file=2067428#file2067428line43>
> >
> > I'd prefer this as being private, with having a package private setter method.
Done.
> On Aug. 8, 2018, 9:57 a.m., Peter Bacsko wrote:
> > sharelib/git/src/main/java/org/apache/oozie/action/hadoop/GitOperations.java
> > Lines 125 (patched)
> > <https://reviews.apache.org/r/59620/diff/10/?file=2067429#file2067429line125>
> >
> > This string can be placed directly in the constructor.
Done.
> On Aug. 8, 2018, 9:57 a.m., Peter Bacsko wrote:
> > sharelib/git/src/test/java/org/apache/oozie/action/hadoop/GitServer.java
> > Lines 74 (patched)
> > <https://reviews.apache.org/r/59620/diff/10/?file=2067430#file2067430line74>
> >
> > At least a warning/error message would be good here.
Done.
- András
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59620/#review206974
-----------------------------------------------------------
On Aug. 3, 2018, 10 p.m., Clay B. wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59620/
> -----------------------------------------------------------
>
> (Updated Aug. 3, 2018, 10 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 b69d2c9
> 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
> fluent-job/fluent-job-api/pom.xml 4c9b853
> fluent-job/fluent-job-api/src/main/java/org/apache/oozie/fluentjob/api/action/GitAction.java PRE-CREATION
> fluent-job/fluent-job-api/src/main/java/org/apache/oozie/fluentjob/api/action/GitActionBuilder.java PRE-CREATION
> fluent-job/fluent-job-api/src/main/java/org/apache/oozie/fluentjob/api/mapping/ExplicitNodeConverter.java 7bb82e5
> fluent-job/fluent-job-api/src/main/java/org/apache/oozie/fluentjob/api/mapping/GitConfigurationConverter.java PRE-CREATION
> fluent-job/fluent-job-api/src/main/java/org/apache/oozie/fluentjob/api/mapping/GitPrepareConverter.java PRE-CREATION
> fluent-job/fluent-job-api/src/main/java/org/apache/oozie/fluentjob/api/serialization/WorkflowMarshaller.java ec56554
> fluent-job/fluent-job-api/src/main/resources/action_mappings.xml a5f890e
> fluent-job/fluent-job-api/src/main/xjb/bindings.xml 48f6890
> pom.xml 92358aa
> 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/GitServer.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/TestGitMain.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 fd3f89f
>
>
> Diff: https://reviews.apache.org/r/59620/diff/10/
>
>
> 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.
>
>
Re: Review Request 59620: This review board request is for an action
to provide a Git action for Oozie
Posted by Peter Bacsko via Review Board <no...@reviews.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59620/#review206974
-----------------------------------------------------------
core/src/main/java/org/apache/oozie/action/hadoop/GitActionExecutor.java
Lines 152 (patched)
<https://reviews.apache.org/r/59620/#comment290133>
Better naming suggested: ActionConfVerifier
core/src/main/java/org/apache/oozie/action/hadoop/GitActionExecutor.java
Lines 169 (patched)
<https://reviews.apache.org/r/59620/#comment290134>
Check visibility of this class & methods (package private OK?)
sharelib/git/src/main/java/org/apache/oozie/action/hadoop/GitMain.java
Lines 38 (patched)
<https://reviews.apache.org/r/59620/#comment290136>
General thoughts: this class has quite a few protected methods. We have to think about whether it's necessary or not. If the class is not going to be subclassed, making them "private" is preferable.
sharelib/git/src/main/java/org/apache/oozie/action/hadoop/GitMain.java
Lines 43 (patched)
<https://reviews.apache.org/r/59620/#comment290135>
I'd prefer this as being private, with having a package private setter method.
sharelib/git/src/main/java/org/apache/oozie/action/hadoop/GitOperations.java
Lines 125 (patched)
<https://reviews.apache.org/r/59620/#comment290137>
This string can be placed directly in the constructor.
sharelib/git/src/test/java/org/apache/oozie/action/hadoop/GitServer.java
Lines 74 (patched)
<https://reviews.apache.org/r/59620/#comment290138>
At least a warning/error message would be good here.
- Peter Bacsko
On aug. 3, 2018, 10 du, Clay B. wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59620/
> -----------------------------------------------------------
>
> (Updated aug. 3, 2018, 10 du)
>
>
> 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 b69d2c9
> 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
> fluent-job/fluent-job-api/pom.xml 4c9b853
> fluent-job/fluent-job-api/src/main/java/org/apache/oozie/fluentjob/api/action/GitAction.java PRE-CREATION
> fluent-job/fluent-job-api/src/main/java/org/apache/oozie/fluentjob/api/action/GitActionBuilder.java PRE-CREATION
> fluent-job/fluent-job-api/src/main/java/org/apache/oozie/fluentjob/api/mapping/ExplicitNodeConverter.java 7bb82e5
> fluent-job/fluent-job-api/src/main/java/org/apache/oozie/fluentjob/api/mapping/GitConfigurationConverter.java PRE-CREATION
> fluent-job/fluent-job-api/src/main/java/org/apache/oozie/fluentjob/api/mapping/GitPrepareConverter.java PRE-CREATION
> fluent-job/fluent-job-api/src/main/java/org/apache/oozie/fluentjob/api/serialization/WorkflowMarshaller.java ec56554
> fluent-job/fluent-job-api/src/main/resources/action_mappings.xml a5f890e
> fluent-job/fluent-job-api/src/main/xjb/bindings.xml 48f6890
> pom.xml 92358aa
> 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/GitServer.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/TestGitMain.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 fd3f89f
>
>
> Diff: https://reviews.apache.org/r/59620/diff/10/
>
>
> 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.
>
>
Re: Review Request 59620: This review board request is for an action
to provide a Git action for Oozie
Posted by "Clay B." <cw...@clayb.net>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59620/
-----------------------------------------------------------
(Updated Aug. 3, 2018, 10 p.m.)
Review request for oozie and András Piros.
Changes
-------
This patch addresses a number of JavaDoc issues. There are still a ton of `@throws` complaints; is it best to document unchecked exceptions?
Bugs: OOZIE-2877
https://issues.apache.org/jira/browse/OOZIE-2877
Repository: oozie-git
Description
-------
OOZIE-2877 - Oozie Git Action
Diffs (updated)
-----
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 b69d2c9
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
fluent-job/fluent-job-api/pom.xml 4c9b853
fluent-job/fluent-job-api/src/main/java/org/apache/oozie/fluentjob/api/action/GitAction.java PRE-CREATION
fluent-job/fluent-job-api/src/main/java/org/apache/oozie/fluentjob/api/action/GitActionBuilder.java PRE-CREATION
fluent-job/fluent-job-api/src/main/java/org/apache/oozie/fluentjob/api/mapping/ExplicitNodeConverter.java 7bb82e5
fluent-job/fluent-job-api/src/main/java/org/apache/oozie/fluentjob/api/mapping/GitConfigurationConverter.java PRE-CREATION
fluent-job/fluent-job-api/src/main/java/org/apache/oozie/fluentjob/api/mapping/GitPrepareConverter.java PRE-CREATION
fluent-job/fluent-job-api/src/main/java/org/apache/oozie/fluentjob/api/serialization/WorkflowMarshaller.java ec56554
fluent-job/fluent-job-api/src/main/resources/action_mappings.xml a5f890e
fluent-job/fluent-job-api/src/main/xjb/bindings.xml 48f6890
pom.xml 92358aa
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/GitServer.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/TestGitMain.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 fd3f89f
Diff: https://reviews.apache.org/r/59620/diff/10/
Changes: https://reviews.apache.org/r/59620/diff/9-10/
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.
Re: Review Request 59620: This review board request is for an action
to provide a Git action for Oozie
Posted by "Clay B." <cw...@clayb.net>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59620/
-----------------------------------------------------------
(Updated Aug. 3, 2018, 8:24 p.m.)
Review request for oozie and András Piros.
Changes
-------
This provides a unit-test Git server, Fluent API support and checks recent changes to isValidUri() among other requests for change.
Bugs: OOZIE-2877
https://issues.apache.org/jira/browse/OOZIE-2877
Repository: oozie-git
Description
-------
OOZIE-2877 - Oozie Git Action
Diffs (updated)
-----
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
fluent-job/fluent-job-api/pom.xml 4c9b853
fluent-job/fluent-job-api/src/main/java/org/apache/oozie/fluentjob/api/action/GitAction.java PRE-CREATION
fluent-job/fluent-job-api/src/main/java/org/apache/oozie/fluentjob/api/action/GitActionBuilder.java PRE-CREATION
fluent-job/fluent-job-api/src/main/java/org/apache/oozie/fluentjob/api/mapping/ExplicitNodeConverter.java 7bb82e5
fluent-job/fluent-job-api/src/main/java/org/apache/oozie/fluentjob/api/mapping/GitConfigurationConverter.java PRE-CREATION
fluent-job/fluent-job-api/src/main/java/org/apache/oozie/fluentjob/api/mapping/GitPrepareConverter.java PRE-CREATION
fluent-job/fluent-job-api/src/main/java/org/apache/oozie/fluentjob/api/serialization/WorkflowMarshaller.java ec56554
fluent-job/fluent-job-api/src/main/resources/action_mappings.xml a5f890e
fluent-job/fluent-job-api/src/main/xjb/bindings.xml 48f6890
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/GitServer.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/TestGitMain.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/9/
Changes: https://reviews.apache.org/r/59620/diff/8-9/
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.
Re: Review Request 59620: This review board request is for an action
to provide a Git action for Oozie
Posted by "Clay B." <cw...@clayb.net>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59620/
-----------------------------------------------------------
(Updated July 30, 2018, 12:56 a.m.)
Review request for oozie and András Piros.
Changes
-------
Matches patch 14 on the JIRA
Bugs: OOZIE-2877
https://issues.apache.org/jira/browse/OOZIE-2877
Repository: oozie-git
Description
-------
OOZIE-2877 - Oozie Git Action
Diffs (updated)
-----
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/GitServer.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/TestGitMain.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/8/
Changes: https://reviews.apache.org/r/59620/diff/7-8/
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.
Re: Review Request 59620: This review board request is for an action
to provide a Git action for Oozie
Posted by "Clay B." <cw...@clayb.net>.
> On July 2, 2018, 1:04 p.m., Peter Bacsko wrote:
> > core/src/main/java/org/apache/oozie/action/hadoop/GitActionExecutor.java
> > Lines 121-127 (patched)
> > <https://reviews.apache.org/r/59620/diff/7/?file=2046211#file2046211line121>
> >
> > Why are we setting/checking these low level properties? Eg. workflow ID is already set at this point. Callback URL ditto, it's handled by JavaActionExecutor.
> >
> > The order of parameters passed to checkAndSet() is also confusing, it should be key, then value, not the other way around.
I needed to leave the namenode setting in and of course the Git-action specific ones. I have refactored the `check*` methods to be key, value instead of the otherway around.
- Clay
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59620/#review205637
-----------------------------------------------------------
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.
>
>
Re: Review Request 59620: This review board request is for an action
to provide a Git action for Oozie
Posted by Peter Bacsko via Review Board <no...@reviews.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59620/#review205637
-----------------------------------------------------------
core/src/main/java/org/apache/oozie/action/hadoop/GitActionExecutor.java
Lines 121-127 (patched)
<https://reviews.apache.org/r/59620/#comment288522>
Why are we setting/checking these low level properties? Eg. workflow ID is already set at this point. Callback URL ditto, it's handled by JavaActionExecutor.
The order of parameters passed to checkAndSet() is also confusing, it should be key, then value, not the other way around.
- Peter Bacsko
On jún. 26, 2018, 9:50 du, Clay B. wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59620/
> -----------------------------------------------------------
>
> (Updated jún. 26, 2018, 9:50 du)
>
>
> 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.
>
>
Re: Review Request 59620: This review board request is for an action
to provide a Git action for Oozie
Posted by "Clay B." <cw...@clayb.net>.
> On July 2, 2018, 12:46 p.m., Peter Bacsko wrote:
> > core/src/main/java/org/apache/oozie/action/hadoop/GitActionExecutor.java
> > Lines 95 (patched)
> > <https://reviews.apache.org/r/59620/diff/7/?file=2046211#file2046211line95>
> >
> > Is this package private for a reason?
No, simply omission. Thanks for finding this!
> On July 2, 2018, 12:46 p.m., Peter Bacsko wrote:
> > docs/src/site/twiki/WorkflowFunctionalSpec.twiki
> > Lines 1667-1669 (patched)
> > <https://reviews.apache.org/r/59620/diff/7/?file=2046214#file2046214line1667>
> >
> > Do we really need to access a different cluster other than the one we're running on?
> >
> > If key is located on a separate cluster which is secure we won't be allowed to access that file without proper credentials. Basically it's the same problem that appears under disctp. It's really not self-explanatory to setup cross-cluster authentication.
> >
> > Therefore I'm against it. Also, the name node is available from "fs.defaultFS" property.
I could certainly see someone storing a credential on a secure object store (e.g. S3-like FS).
> On July 2, 2018, 12:46 p.m., Peter Bacsko wrote:
> > sharelib/git/src/main/java/org/apache/oozie/action/hadoop/GitMain.java
> > Lines 41 (patched)
> > <https://reviews.apache.org/r/59620/diff/7/?file=2046219#file2046219line41>
> >
> > We don't use XLog inside LauncherMain, only Sysout. This will change in the future, but for now, let's stick to the conventions that we have already.
I have left the help methods to leave logging level intention in place but removed all references to XLog. Would you prefer the helpers to go too?
> On July 2, 2018, 12:46 p.m., Peter Bacsko wrote:
> > sharelib/git/src/main/java/org/apache/oozie/action/hadoop/GitMain.java
> > Lines 166 (patched)
> > <https://reviews.apache.org/r/59620/diff/7/?file=2046219#file2046219line166>
> >
> > A better method name would be something like isValidUri()
Sounds good
> On July 2, 2018, 12:46 p.m., Peter Bacsko wrote:
> > sharelib/git/src/main/java/org/apache/oozie/action/hadoop/GitMain.java
> > Lines 185-188 (patched)
> > <https://reviews.apache.org/r/59620/diff/7/?file=2046219#file2046219line185>
> >
> > Is it necessary to validate these separately? If anything, these values should be checked inside Oozie, not when the container is alreay running.
I have removed these checks.
> On July 2, 2018, 12:46 p.m., Peter Bacsko wrote:
> > sharelib/git/src/main/java/org/apache/oozie/action/hadoop/GitMain.java
> > Lines 201-204 (patched)
> > <https://reviews.apache.org/r/59620/diff/7/?file=2046219#file2046219line201>
> >
> > If we don't allow relative URIs (null scheme), then this check should be placed inside validUri().
Integrated into isValidURI.
> On July 2, 2018, 12:46 p.m., Peter Bacsko wrote:
> > sharelib/git/src/main/java/org/apache/oozie/action/hadoop/GitOperations.java
> > Lines 82-99 (patched)
> > <https://reviews.apache.org/r/59620/diff/7/?file=2046220#file2046220line82>
> >
> > We should not clone repositories under an unit test. I suggest using a Jsch mock for testing.
> >
> > There are different ways to achieve this. I can think of having a property like "oozie.git.operations.class" which defaults to this class but we can override it to a different implementation which does not use JSch.
I have found a few descriptions on how to build a unit-test server in JGit; now doing that.
> On July 2, 2018, 12:46 p.m., Peter Bacsko wrote:
> > sharelib/git/src/test/java/org/apache/oozie/action/hadoop/TestIntegrationGitActionExecutor.java
> > Lines 43 (patched)
> > <https://reviews.apache.org/r/59620/diff/7/?file=2046223#file2046223line43>
> >
> > As I mentioned above, let's avoid cloning a repository during test execution.
Still clones but from an in-JVM server.
> On July 2, 2018, 12:46 p.m., Peter Bacsko wrote:
> > sharelib/git/src/test/java/org/apache/oozie/action/hadoop/TestIntegrationGitActionExecutor.java
> > Lines 119-122 (patched)
> > <https://reviews.apache.org/r/59620/diff/7/?file=2046223#file2046223line119>
> >
> > Three things:
> >
> > 1. We don't need JobConf anymore because we don't launch the action as a MapReduce action. Just use the plain Configuration class which JobConf extends.
> >
> > 2. No need to set "mapred.job.tracker", it's a deprecated property anyway
> >
> > 3. createJobClient() creates an MR job client, but we don't need it, plus the return value is ignored.
I still use createJobConf from XTestCase to keep uniformity with other test cases but I use a Configuration instead of a JobConf now.
- Clay
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59620/#review205636
-----------------------------------------------------------
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.
>
>
Re: Review Request 59620: This review board request is for an action
to provide a Git action for Oozie
Posted by Peter Bacsko via Review Board <no...@reviews.apache.org>.
> On júl. 2, 2018, 12:46 du, Peter Bacsko wrote:
> > docs/src/site/twiki/WorkflowFunctionalSpec.twiki
> > Lines 1667-1669 (patched)
> > <https://reviews.apache.org/r/59620/diff/7/?file=2046214#file2046214line1667>
> >
> > Do we really need to access a different cluster other than the one we're running on?
> >
> > If key is located on a separate cluster which is secure we won't be allowed to access that file without proper credentials. Basically it's the same problem that appears under disctp. It's really not self-explanatory to setup cross-cluster authentication.
> >
> > Therefore I'm against it. Also, the name node is available from "fs.defaultFS" property.
>
> Clay B. wrote:
> I could certainly see someone storing a credential on a secure object store (e.g. S3-like FS).
In that case, we have to mention in the docs that access control on the remote side must be properly configured. Eg. if it's a hdfs:// URL that points to an non-secure cluster, appropriate HDFS permissions are still necessary.
- Peter
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59620/#review205636
-----------------------------------------------------------
On aug. 3, 2018, 10 du, Clay B. wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59620/
> -----------------------------------------------------------
>
> (Updated aug. 3, 2018, 10 du)
>
>
> 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 b69d2c9
> 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
> fluent-job/fluent-job-api/pom.xml 4c9b853
> fluent-job/fluent-job-api/src/main/java/org/apache/oozie/fluentjob/api/action/GitAction.java PRE-CREATION
> fluent-job/fluent-job-api/src/main/java/org/apache/oozie/fluentjob/api/action/GitActionBuilder.java PRE-CREATION
> fluent-job/fluent-job-api/src/main/java/org/apache/oozie/fluentjob/api/mapping/ExplicitNodeConverter.java 7bb82e5
> fluent-job/fluent-job-api/src/main/java/org/apache/oozie/fluentjob/api/mapping/GitConfigurationConverter.java PRE-CREATION
> fluent-job/fluent-job-api/src/main/java/org/apache/oozie/fluentjob/api/mapping/GitPrepareConverter.java PRE-CREATION
> fluent-job/fluent-job-api/src/main/java/org/apache/oozie/fluentjob/api/serialization/WorkflowMarshaller.java ec56554
> fluent-job/fluent-job-api/src/main/resources/action_mappings.xml a5f890e
> fluent-job/fluent-job-api/src/main/xjb/bindings.xml 48f6890
> pom.xml 92358aa
> 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/GitServer.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/TestGitMain.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 fd3f89f
>
>
> Diff: https://reviews.apache.org/r/59620/diff/10/
>
>
> 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.
>
>
Re: Review Request 59620: This review board request is for an action
to provide a Git action for Oozie
Posted by Peter Bacsko via Review Board <no...@reviews.apache.org>.
> On júl. 2, 2018, 12:46 du, Peter Bacsko wrote:
> > sharelib/git/src/main/java/org/apache/oozie/action/hadoop/GitMain.java
> > Lines 41 (patched)
> > <https://reviews.apache.org/r/59620/diff/7/?file=2046219#file2046219line41>
> >
> > We don't use XLog inside LauncherMain, only Sysout. This will change in the future, but for now, let's stick to the conventions that we have already.
>
> Clay B. wrote:
> I have left the help methods to leave logging level intention in place but removed all references to XLog. Would you prefer the helpers to go too?
We haven't settled down on a logging solution. Geza suggested JDK loggers (so that they don't conflict with log4j/log4j2/logback, whatever we migrate to), but it's not finalized yet.
- Peter
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59620/#review205636
-----------------------------------------------------------
On aug. 3, 2018, 10 du, Clay B. wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59620/
> -----------------------------------------------------------
>
> (Updated aug. 3, 2018, 10 du)
>
>
> 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 b69d2c9
> 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
> fluent-job/fluent-job-api/pom.xml 4c9b853
> fluent-job/fluent-job-api/src/main/java/org/apache/oozie/fluentjob/api/action/GitAction.java PRE-CREATION
> fluent-job/fluent-job-api/src/main/java/org/apache/oozie/fluentjob/api/action/GitActionBuilder.java PRE-CREATION
> fluent-job/fluent-job-api/src/main/java/org/apache/oozie/fluentjob/api/mapping/ExplicitNodeConverter.java 7bb82e5
> fluent-job/fluent-job-api/src/main/java/org/apache/oozie/fluentjob/api/mapping/GitConfigurationConverter.java PRE-CREATION
> fluent-job/fluent-job-api/src/main/java/org/apache/oozie/fluentjob/api/mapping/GitPrepareConverter.java PRE-CREATION
> fluent-job/fluent-job-api/src/main/java/org/apache/oozie/fluentjob/api/serialization/WorkflowMarshaller.java ec56554
> fluent-job/fluent-job-api/src/main/resources/action_mappings.xml a5f890e
> fluent-job/fluent-job-api/src/main/xjb/bindings.xml 48f6890
> pom.xml 92358aa
> 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/GitServer.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/TestGitMain.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 fd3f89f
>
>
> Diff: https://reviews.apache.org/r/59620/diff/10/
>
>
> 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.
>
>
Re: Review Request 59620: This review board request is for an action
to provide a Git action for Oozie
Posted by András Piros via Review Board <no...@reviews.apache.org>.
> On July 2, 2018, 12:46 p.m., Peter Bacsko wrote:
> > docs/src/site/twiki/WorkflowFunctionalSpec.twiki
> > Lines 1667-1669 (patched)
> > <https://reviews.apache.org/r/59620/diff/7/?file=2046214#file2046214line1667>
> >
> > Do we really need to access a different cluster other than the one we're running on?
> >
> > If key is located on a separate cluster which is secure we won't be allowed to access that file without proper credentials. Basically it's the same problem that appears under disctp. It's really not self-explanatory to setup cross-cluster authentication.
> >
> > Therefore I'm against it. Also, the name node is available from "fs.defaultFS" property.
>
> Clay B. wrote:
> I could certainly see someone storing a credential on a secure object store (e.g. S3-like FS).
>
> Peter Bacsko wrote:
> In that case, we have to mention in the docs that access control on the remote side must be properly configured. Eg. if it's a hdfs:// URL that points to an non-secure cluster, appropriate HDFS permissions are still necessary.
A paragraph added to the docs.
> On July 2, 2018, 12:46 p.m., Peter Bacsko wrote:
> > sharelib/git/src/main/java/org/apache/oozie/action/hadoop/GitMain.java
> > Lines 41 (patched)
> > <https://reviews.apache.org/r/59620/diff/7/?file=2046219#file2046219line41>
> >
> > We don't use XLog inside LauncherMain, only Sysout. This will change in the future, but for now, let's stick to the conventions that we have already.
>
> Clay B. wrote:
> I have left the help methods to leave logging level intention in place but removed all references to XLog. Would you prefer the helpers to go too?
>
> Peter Bacsko wrote:
> We haven't settled down on a logging solution. Geza suggested JDK loggers (so that they don't conflict with log4j/log4j2/logback, whatever we migrate to), but it's not finalized yet.
`XLog` references are not more present in `GitMain`.
- András
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59620/#review205636
-----------------------------------------------------------
On Aug. 3, 2018, 10 p.m., Clay B. wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59620/
> -----------------------------------------------------------
>
> (Updated Aug. 3, 2018, 10 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 b69d2c9
> 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
> fluent-job/fluent-job-api/pom.xml 4c9b853
> fluent-job/fluent-job-api/src/main/java/org/apache/oozie/fluentjob/api/action/GitAction.java PRE-CREATION
> fluent-job/fluent-job-api/src/main/java/org/apache/oozie/fluentjob/api/action/GitActionBuilder.java PRE-CREATION
> fluent-job/fluent-job-api/src/main/java/org/apache/oozie/fluentjob/api/mapping/ExplicitNodeConverter.java 7bb82e5
> fluent-job/fluent-job-api/src/main/java/org/apache/oozie/fluentjob/api/mapping/GitConfigurationConverter.java PRE-CREATION
> fluent-job/fluent-job-api/src/main/java/org/apache/oozie/fluentjob/api/mapping/GitPrepareConverter.java PRE-CREATION
> fluent-job/fluent-job-api/src/main/java/org/apache/oozie/fluentjob/api/serialization/WorkflowMarshaller.java ec56554
> fluent-job/fluent-job-api/src/main/resources/action_mappings.xml a5f890e
> fluent-job/fluent-job-api/src/main/xjb/bindings.xml 48f6890
> pom.xml 92358aa
> 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/GitServer.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/TestGitMain.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 fd3f89f
>
>
> Diff: https://reviews.apache.org/r/59620/diff/10/
>
>
> 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.
>
>
Re: Review Request 59620: This review board request is for an action
to provide a Git action for Oozie
Posted by Peter Bacsko via Review Board <no...@reviews.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59620/#review205636
-----------------------------------------------------------
core/src/main/java/org/apache/oozie/action/hadoop/GitActionExecutor.java
Lines 95 (patched)
<https://reviews.apache.org/r/59620/#comment288510>
Is this package private for a reason?
core/src/main/java/org/apache/oozie/action/hadoop/GitActionExecutor.java
Lines 226-227 (patched)
<https://reviews.apache.org/r/59620/#comment288509>
We shouldn't catch NPE let alone ignore it.
docs/src/site/twiki/WorkflowFunctionalSpec.twiki
Lines 1667-1669 (patched)
<https://reviews.apache.org/r/59620/#comment288515>
Do we really need to access a different cluster other than the one we're running on?
If key is located on a separate cluster which is secure we won't be allowed to access that file without proper credentials. Basically it's the same problem that appears under disctp. It's really not self-explanatory to setup cross-cluster authentication.
Therefore I'm against it. Also, the name node is available from "fs.defaultFS" property.
sharelib/git/src/main/java/org/apache/oozie/action/hadoop/GitMain.java
Lines 41 (patched)
<https://reviews.apache.org/r/59620/#comment288508>
We don't use XLog inside LauncherMain, only Sysout. This will change in the future, but for now, let's stick to the conventions that we have already.
sharelib/git/src/main/java/org/apache/oozie/action/hadoop/GitMain.java
Lines 166 (patched)
<https://reviews.apache.org/r/59620/#comment288513>
A better method name would be something like isValidUri()
sharelib/git/src/main/java/org/apache/oozie/action/hadoop/GitMain.java
Lines 185-188 (patched)
<https://reviews.apache.org/r/59620/#comment288512>
Is it necessary to validate these separately? If anything, these values should be checked inside Oozie, not when the container is alreay running.
sharelib/git/src/main/java/org/apache/oozie/action/hadoop/GitMain.java
Lines 201-204 (patched)
<https://reviews.apache.org/r/59620/#comment288514>
If we don't allow relative URIs (null scheme), then this check should be placed inside validUri().
sharelib/git/src/main/java/org/apache/oozie/action/hadoop/GitOperations.java
Lines 54-56 (patched)
<https://reviews.apache.org/r/59620/#comment288516>
Would be better to have these as final
sharelib/git/src/main/java/org/apache/oozie/action/hadoop/GitOperations.java
Lines 82-99 (patched)
<https://reviews.apache.org/r/59620/#comment288517>
We should not clone repositories under an unit test. I suggest using a Jsch mock for testing.
There are different ways to achieve this. I can think of having a property like "oozie.git.operations.class" which defaults to this class but we can override it to a different implementation which does not use JSch.
sharelib/git/src/test/java/org/apache/oozie/action/hadoop/TestGitActionExecutor.java
Lines 113 (patched)
<https://reviews.apache.org/r/59620/#comment288506>
Use Assert.fail() to indicate failure
sharelib/git/src/test/java/org/apache/oozie/action/hadoop/TestGitActionExecutor.java
Lines 116 (patched)
<https://reviews.apache.org/r/59620/#comment288507>
Assert.fail()
sharelib/git/src/test/java/org/apache/oozie/action/hadoop/TestGitMainGetKey.java
Lines 51-56 (patched)
<https://reviews.apache.org/r/59620/#comment288520>
Simplify this part, see related comment below
sharelib/git/src/test/java/org/apache/oozie/action/hadoop/TestIntegrationGitActionExecutor.java
Lines 43 (patched)
<https://reviews.apache.org/r/59620/#comment288518>
As I mentioned above, let's avoid cloning a repository during test execution.
sharelib/git/src/test/java/org/apache/oozie/action/hadoop/TestIntegrationGitActionExecutor.java
Lines 74-80 (patched)
<https://reviews.apache.org/r/59620/#comment288519>
This is way too complicated. Do we just read a contents of a file?
There are simpler means to do this: https://howtodoinjava.com/core-java/io/java-read-file-to-string-examples/
sharelib/git/src/test/java/org/apache/oozie/action/hadoop/TestIntegrationGitActionExecutor.java
Lines 119-122 (patched)
<https://reviews.apache.org/r/59620/#comment288521>
Three things:
1. We don't need JobConf anymore because we don't launch the action as a MapReduce action. Just use the plain Configuration class which JobConf extends.
2. No need to set "mapred.job.tracker", it's a deprecated property anyway
3. createJobClient() creates an MR job client, but we don't need it, plus the return value is ignored.
- Peter Bacsko
On jún. 26, 2018, 9:50 du, Clay B. wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59620/
> -----------------------------------------------------------
>
> (Updated jún. 26, 2018, 9:50 du)
>
>
> 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.
>
>
Re: Review Request 59620: This review board request is for an action
to provide a Git action for Oozie
Posted by "Clay B." <cw...@clayb.net>.
-----------------------------------------------------------
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.
Changes
-------
Upload of OOZIE-2877.013-1.patch
Bugs: OOZIE-2877
https://issues.apache.org/jira/browse/OOZIE-2877
Repository: oozie-git
Description
-------
OOZIE-2877 - Oozie Git Action
Diffs (updated)
-----
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/
Changes: https://reviews.apache.org/r/59620/diff/6-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.
Re: Review Request 59620: This review board request is for an action
to provide a Git action for Oozie
Posted by "Clay B." <cw...@clayb.net>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59620/
-----------------------------------------------------------
(Updated June 4, 2018, 5:44 a.m.)
Review request for oozie and András Piros.
Changes
-------
Latest set of changes addressing (nearly all) comments from three weeks ago
Bugs: OOZIE-2877
https://issues.apache.org/jira/browse/OOZIE-2877
Repository: oozie-git
Description
-------
OOZIE-2877 - Oozie Git Action
Diffs (updated)
-----
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 c54db34
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 6f35868
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 67526d9
Diff: https://reviews.apache.org/r/59620/diff/6/
Changes: https://reviews.apache.org/r/59620/diff/5-6/
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.
Re: Review Request 59620: This review board request is for an action
to provide a Git action for Oozie
Posted by "Clay B." <cw...@clayb.net>.
> On May 7, 2018, 12:13 p.m., Peter Cseh wrote:
> > core/src/main/java/org/apache/oozie/action/hadoop/GitActionExecutor.java
> > Lines 55-56 (patched)
> > <https://reviews.apache.org/r/59620/diff/5/?file=2016895#file2016895line55>
> >
> > oozie.oozie? seems redundant :)
Ah yes, I see now.
- Clay
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59620/#review202545
-----------------------------------------------------------
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.
>
>
Re: Review Request 59620: This review board request is for an action
to provide a Git action for Oozie
Posted by Peter Cseh via Review Board <no...@reviews.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59620/#review202545
-----------------------------------------------------------
core/src/main/java/org/apache/oozie/action/hadoop/GitActionExecutor.java
Lines 55-56 (patched)
<https://reviews.apache.org/r/59620/#comment284363>
oozie.oozie? seems redundant :)
- Peter Cseh
On May 4, 2018, 5:43 p.m., Clay B. wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59620/
> -----------------------------------------------------------
>
> (Updated May 4, 2018, 5:43 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/java/org/apache/oozie/cli/OozieCLI.java 4abc7502c0c9d8b59ded2baaed30c407ad073008
> 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 6f8925828090cee29a818de30a7c31db0617d816
> docs/src/site/twiki/WorkflowFunctionalSpec.twiki 76cbe21ecd79eae86885a8db77831bf2c40f2d4f
> examples/src/main/apps/git/job.properties PRE-CREATION
> examples/src/main/apps/git/workflow.xml PRE-CREATION
> pom.xml d9fe1b20f571b1a6afacb3ce53f4d409dd3d60b1
> 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 6a0864d4e5b52160dc4e1a5cb9dfc34a037b67c8
> src/main/assemblies/sharelib.xml 07dc69c8276895b254e5af0cc021cce6ebad18f4
> webapp/pom.xml 797996912b6e6381b261a69f8eb1e012fe488fdf
>
>
> Diff: https://reviews.apache.org/r/59620/diff/5/
>
>
> 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.
>
>
Re: Review Request 59620: This review board request is for an action
to provide a Git action for Oozie
Posted by "Clay B." <cw...@clayb.net>.
> On May 7, 2018, 12:14 p.m., Peter Cseh wrote:
> > examples/src/main/apps/git/workflow.xml
> > Lines 24-25 (patched)
> > <https://reviews.apache.org/r/59620/diff/5/?file=2016899#file2016899line24>
> >
> > Let's check out apache/Oozie from github instead :)
Yes, I had an interim smallish repo. Unfortunately, JGit seems to have issues with branches and repo depth at this time -- two features that will really help to add in the Git action once it gets off the ground. A full checkout of Oozie over even a reasonable coffee shop wifi (~200 KiB/s) can be ~5+ minutes so we should use something small. Andras suggested Scoozi in his patches but I would prefer something in Apache. Any Apache suggestions for repos which might be smaller?
- Clay
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59620/#review202546
-----------------------------------------------------------
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.
>
>
Re: Review Request 59620: This review board request is for an action
to provide a Git action for Oozie
Posted by Peter Cseh via Review Board <no...@reviews.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59620/#review202546
-----------------------------------------------------------
examples/src/main/apps/git/workflow.xml
Lines 24-25 (patched)
<https://reviews.apache.org/r/59620/#comment284367>
Let's check out apache/Oozie from github instead :)
- Peter Cseh
On May 4, 2018, 5:43 p.m., Clay B. wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59620/
> -----------------------------------------------------------
>
> (Updated May 4, 2018, 5:43 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/java/org/apache/oozie/cli/OozieCLI.java 4abc7502c0c9d8b59ded2baaed30c407ad073008
> 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 6f8925828090cee29a818de30a7c31db0617d816
> docs/src/site/twiki/WorkflowFunctionalSpec.twiki 76cbe21ecd79eae86885a8db77831bf2c40f2d4f
> examples/src/main/apps/git/job.properties PRE-CREATION
> examples/src/main/apps/git/workflow.xml PRE-CREATION
> pom.xml d9fe1b20f571b1a6afacb3ce53f4d409dd3d60b1
> 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 6a0864d4e5b52160dc4e1a5cb9dfc34a037b67c8
> src/main/assemblies/sharelib.xml 07dc69c8276895b254e5af0cc021cce6ebad18f4
> webapp/pom.xml 797996912b6e6381b261a69f8eb1e012fe488fdf
>
>
> Diff: https://reviews.apache.org/r/59620/diff/5/
>
>
> 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.
>
>
Re: Review Request 59620: This review board request is for an action
to provide a Git action for Oozie
Posted by "Clay B." <cw...@clayb.net>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59620/
-----------------------------------------------------------
(Updated May 4, 2018, 5:43 p.m.)
Review request for oozie and András Piros.
Changes
-------
Andras' Patch 011
Bugs: OOZIE-2877
https://issues.apache.org/jira/browse/OOZIE-2877
Repository: oozie-git
Description
-------
OOZIE-2877 - Oozie Git Action
Diffs (updated)
-----
client/src/main/java/org/apache/oozie/cli/OozieCLI.java 4abc7502c0c9d8b59ded2baaed30c407ad073008
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 6f8925828090cee29a818de30a7c31db0617d816
docs/src/site/twiki/WorkflowFunctionalSpec.twiki 76cbe21ecd79eae86885a8db77831bf2c40f2d4f
examples/src/main/apps/git/job.properties PRE-CREATION
examples/src/main/apps/git/workflow.xml PRE-CREATION
pom.xml d9fe1b20f571b1a6afacb3ce53f4d409dd3d60b1
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 6a0864d4e5b52160dc4e1a5cb9dfc34a037b67c8
src/main/assemblies/sharelib.xml 07dc69c8276895b254e5af0cc021cce6ebad18f4
webapp/pom.xml 797996912b6e6381b261a69f8eb1e012fe488fdf
Diff: https://reviews.apache.org/r/59620/diff/5/
Changes: https://reviews.apache.org/r/59620/diff/4-5/
Testing (updated)
-------
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.
Re: Review Request 59620: This review board request is for an action
to provide a Git action for Oozie
Posted by "Clay B." <cw...@clayb.net>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59620/
-----------------------------------------------------------
(Updated May 4, 2018, 5:40 p.m.)
Review request for oozie and András Piros.
Changes
-------
Andras' Patch 010
Bugs: OOZIE-2877
https://issues.apache.org/jira/browse/OOZIE-2877
Repository: oozie-git
Description
-------
OOZIE-2877 - Oozie Git Action
Diffs (updated)
-----
client/src/main/java/org/apache/oozie/cli/OozieCLI.java 4abc7502c0c9d8b59ded2baaed30c407ad073008
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 6f8925828090cee29a818de30a7c31db0617d816
docs/src/site/twiki/WorkflowFunctionalSpec.twiki 76cbe21ecd79eae86885a8db77831bf2c40f2d4f
examples/src/main/apps/git/job.properties PRE-CREATION
examples/src/main/apps/git/workflow.xml PRE-CREATION
pom.xml d9fe1b20f571b1a6afacb3ce53f4d409dd3d60b1
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 6a0864d4e5b52160dc4e1a5cb9dfc34a037b67c8
src/main/assemblies/sharelib.xml 07dc69c8276895b254e5af0cc021cce6ebad18f4
webapp/pom.xml 797996912b6e6381b261a69f8eb1e012fe488fdf
Diff: https://reviews.apache.org/r/59620/diff/4/
Changes: https://reviews.apache.org/r/59620/diff/3-4/
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.
Re: Review Request 59620: This review board request is for an action
to provide a Git action for Oozie
Posted by "Clay B." <cw...@clayb.net>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59620/
-----------------------------------------------------------
(Updated May 4, 2018, 5:38 p.m.)
Review request for oozie and András Piros.
Changes
-------
My rebased patch from 20/Apr/18 06:38
Bugs: OOZIE-2877
https://issues.apache.org/jira/browse/OOZIE-2877
Repository: oozie-git
Description
-------
OOZIE-2877 - Oozie Git Action
Diffs (updated)
-----
client/src/main/java/org/apache/oozie/cli/OozieCLI.java 4abc750
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 6f89258
docs/src/site/twiki/WorkflowFunctionalSpec.twiki 5df6024
pom.xml d9fe1b2
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 6a0864d
src/main/assemblies/sharelib.xml 07dc69c
webapp/pom.xml 7979969
Diff: https://reviews.apache.org/r/59620/diff/3/
Changes: https://reviews.apache.org/r/59620/diff/2-3/
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.
Re: Review Request 59620: This review board request is for an action
to provide a Git action for Oozie
Posted by András Piros via Review Board <no...@reviews.apache.org>.
> 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?
>
> Clay B. wrote:
> 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?
@Peter Cseh it's already covered by `HadoopAccessorService#createFileSystem()` that to latter end is called by `JavaActionExecutor#setupActionConf()` as well. So no need to cover that separately.
> 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.
>
> Clay B. wrote:
> 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?
@Peter Cseh can you please give an answer? Thanks!
- András
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59620/#review202124
-----------------------------------------------------------
On Aug. 3, 2018, 10 p.m., Clay B. wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59620/
> -----------------------------------------------------------
>
> (Updated Aug. 3, 2018, 10 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 b69d2c9
> 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
> fluent-job/fluent-job-api/pom.xml 4c9b853
> fluent-job/fluent-job-api/src/main/java/org/apache/oozie/fluentjob/api/action/GitAction.java PRE-CREATION
> fluent-job/fluent-job-api/src/main/java/org/apache/oozie/fluentjob/api/action/GitActionBuilder.java PRE-CREATION
> fluent-job/fluent-job-api/src/main/java/org/apache/oozie/fluentjob/api/mapping/ExplicitNodeConverter.java 7bb82e5
> fluent-job/fluent-job-api/src/main/java/org/apache/oozie/fluentjob/api/mapping/GitConfigurationConverter.java PRE-CREATION
> fluent-job/fluent-job-api/src/main/java/org/apache/oozie/fluentjob/api/mapping/GitPrepareConverter.java PRE-CREATION
> fluent-job/fluent-job-api/src/main/java/org/apache/oozie/fluentjob/api/serialization/WorkflowMarshaller.java ec56554
> fluent-job/fluent-job-api/src/main/resources/action_mappings.xml a5f890e
> fluent-job/fluent-job-api/src/main/xjb/bindings.xml 48f6890
> pom.xml 92358aa
> 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/GitServer.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/TestGitMain.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 fd3f89f
>
>
> Diff: https://reviews.apache.org/r/59620/diff/10/
>
>
> 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.
>
>
Re: Review Request 59620: This review board request is for an action
to provide a Git action for Oozie
Posted by "Clay B." <cw...@clayb.net>.
> 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.
>
>
Re: Review Request 59620: This review board request is for an action
to provide a Git action for Oozie
Posted by Peter Cseh via Review Board <no...@reviews.apache.org>.
-----------------------------------------------------------
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.
>
>
Re: Review Request 59620: This review board request is for an action
to provide a Git action for Oozie
Posted by "Clay B." <cw...@clayb.net>.
-----------------------------------------------------------
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.
Changes
-------
Review Board comments responded to so far. Still need Twiki documentation and breaks some `TestHDFSOperats` tests
Bugs: OOZIE-2877
https://issues.apache.org/jira/browse/OOZIE-2877
Repository: oozie-git
Description
-------
OOZIE-2877 - Oozie Git Action
Diffs (updated)
-----
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/
Changes: https://reviews.apache.org/r/59620/diff/1-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.