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 2018/08/03 20:24:53 UTC
Re: 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/
-----------------------------------------------------------
(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 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.