Re: Review Request 70502: OOZIE-3265 properties RERUN_FAIL_NODES and RERUN_SKIP_NODES should be able to appear together

2019-07-17 Thread Kinga Marton via Review Board

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

(Updated July 17, 2019, 11:47 a.m.)


Review request for oozie and Andras Salamon.


Repository: oozie-git


Description
---

Currently when you re-run a workflow with property "oozie.wf.rerun.failnodes"  
set to true,

you can no longer re-run it again with "oozie.wf.rerun.skip.nodes" property 
specified, even if you set "oozie.wf.rerun.failnodes" to false.

This kind of limitation is not reasonable. There is only one case where 
"oozie.wf.rerun.failnodes" is true and "oozie.wf.rerun.skip.nodes" is not null 
or empty, that should be disallowed.


Diffs (updated)
-

  core/src/main/java/org/apache/oozie/DagEngine.java 70ce96122 
  core/src/test/java/org/apache/oozie/TestDagEngine.java 214740574 
  core/src/test/java/org/apache/oozie/command/wf/TestReRunXCommand.java 
d1d75b50a 
  docs/src/site/markdown/DG_WorkflowReRun.md c128681a8 


Diff: https://reviews.apache.org/r/70502/diff/4/

Changes: https://reviews.apache.org/r/70502/diff/3-4/


Testing
---

unit tests added


Thanks,

Kinga Marton



Re: Review Request 70502: OOZIE-3265 properties RERUN_FAIL_NODES and RERUN_SKIP_NODES should be able to appear together

2019-07-17 Thread Kinga Marton via Review Board


> On July 16, 2019, 8:44 a.m., Andras Salamon wrote:
> > docs/src/site/markdown/DG_WorkflowReRun.md
> > Lines 27-28 (original), 29-30 (patched)
> > 
> >
> > What is wfId? Probably the id after -rerun but it's not clear from the 
> > documentation.

I think that the wfId is used only to show that the rerunned workflow will have 
the same id.


- Kinga


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


On July 15, 2019, 12:14 p.m., Kinga Marton wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70502/
> ---
> 
> (Updated July 15, 2019, 12:14 p.m.)
> 
> 
> Review request for oozie and Andras Salamon.
> 
> 
> Repository: oozie-git
> 
> 
> Description
> ---
> 
> Currently when you re-run a workflow with property "oozie.wf.rerun.failnodes" 
>  set to true,
> 
> you can no longer re-run it again with "oozie.wf.rerun.skip.nodes" property 
> specified, even if you set "oozie.wf.rerun.failnodes" to false.
> 
> This kind of limitation is not reasonable. There is only one case where 
> "oozie.wf.rerun.failnodes" is true and "oozie.wf.rerun.skip.nodes" is not 
> null or empty, that should be disallowed.
> 
> 
> Diffs
> -
> 
>   core/src/main/java/org/apache/oozie/DagEngine.java 70ce96122 
>   core/src/test/java/org/apache/oozie/TestDagEngine.java 214740574 
>   core/src/test/java/org/apache/oozie/command/wf/TestReRunXCommand.java 
> d1d75b50a 
>   docs/src/site/markdown/DG_WorkflowReRun.md c128681a8 
> 
> 
> Diff: https://reviews.apache.org/r/70502/diff/3/
> 
> 
> Testing
> ---
> 
> unit tests added
> 
> 
> Thanks,
> 
> Kinga Marton
> 
>



Re: Review Request 70502: OOZIE-3265 properties RERUN_FAIL_NODES and RERUN_SKIP_NODES should be able to appear together

2019-07-15 Thread Kinga Marton via Review Board

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

(Updated July 15, 2019, 12:14 p.m.)


Review request for oozie and Andras Salamon.


Repository: oozie-git


Description
---

Currently when you re-run a workflow with property "oozie.wf.rerun.failnodes"  
set to true,

you can no longer re-run it again with "oozie.wf.rerun.skip.nodes" property 
specified, even if you set "oozie.wf.rerun.failnodes" to false.

This kind of limitation is not reasonable. There is only one case where 
"oozie.wf.rerun.failnodes" is true and "oozie.wf.rerun.skip.nodes" is not null 
or empty, that should be disallowed.


Diffs (updated)
-

  core/src/main/java/org/apache/oozie/DagEngine.java 70ce96122 
  core/src/test/java/org/apache/oozie/TestDagEngine.java 214740574 
  core/src/test/java/org/apache/oozie/command/wf/TestReRunXCommand.java 
d1d75b50a 
  docs/src/site/markdown/DG_WorkflowReRun.md c128681a8 


Diff: https://reviews.apache.org/r/70502/diff/3/

Changes: https://reviews.apache.org/r/70502/diff/2-3/


Testing
---

unit tests added


Thanks,

Kinga Marton



Re: Review Request 70964: OOZIE-3513 Migrate from Preconditions.checkNotNull and ParamChecker.notNull

2019-07-01 Thread Kinga Marton via Review Board

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


Ship it!




Ship It!

- Kinga Marton


On June 28, 2019, 8:57 a.m., Andras Salamon wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70964/
> ---
> 
> (Updated June 28, 2019, 8:57 a.m.)
> 
> 
> Review request for oozie and Kinga Marton.
> 
> 
> Repository: oozie-git
> 
> 
> Description
> ---
> 
> OOZIE-3513 Migrate from Preconditions.checkNotNull and ParamChecker.notNull
> 
> 
> Diffs
> -
> 
>   client/src/main/java/org/apache/oozie/client/ApiJarFactory.java bb85d6780 
>   client/src/main/java/org/apache/oozie/client/ApiJarLoader.java 6ecf485ed 
>   core/src/main/java/org/apache/oozie/BaseLocalOozieClient.java 376ea1165 
>   core/src/main/java/org/apache/oozie/XException.java fb1a381bb 
>   core/src/main/java/org/apache/oozie/action/ActionExecutorException.java 
> 9bc0e81b9 
>   core/src/main/java/org/apache/oozie/action/hadoop/GitActionExecutor.java 
> fc4d65b83 
>   core/src/main/java/org/apache/oozie/action/hadoop/HadoopTokenHelper.java 
> 1bdeb32f6 
>   core/src/main/java/org/apache/oozie/action/hadoop/JavaActionExecutor.java 
> ec45fe495 
>   
> core/src/main/java/org/apache/oozie/action/hadoop/MapReduceActionExecutor.java
>  a509e4d98 
>   core/src/main/java/org/apache/oozie/action/hadoop/ShareLibExcluder.java 
> 9c45e5392 
>   core/src/main/java/org/apache/oozie/command/TransitionXCommand.java 
> 262a78f5b 
>   
> core/src/main/java/org/apache/oozie/command/bundle/BundleJobResumeXCommand.java
>  bc58be6ef 
>   
> core/src/main/java/org/apache/oozie/command/bundle/BundleSubmitXCommand.java 
> ab88cac6c 
>   
> core/src/main/java/org/apache/oozie/command/coord/CoordActionNotificationXCommand.java
>  d51f0d780 
>   
> core/src/main/java/org/apache/oozie/command/coord/CoordActionSkipXCommand.java
>  ef0fc2d3e 
>   
> core/src/main/java/org/apache/oozie/command/coord/CoordActionTimeOutXCommand.java
>  9646d738a 
>   
> core/src/main/java/org/apache/oozie/command/coord/CoordActionsIgnoreXCommand.java
>  88842f1f1 
>   core/src/main/java/org/apache/oozie/command/coord/CoordRerunXCommand.java 
> 2f158cdbf 
>   core/src/main/java/org/apache/oozie/command/coord/CoordSubmitXCommand.java 
> 2622f05d9 
>   core/src/main/java/org/apache/oozie/command/wf/ReRunXCommand.java dd8d3d2f6 
>   core/src/main/java/org/apache/oozie/command/wf/SubmitHttpXCommand.java 
> 9ee84063a 
>   core/src/main/java/org/apache/oozie/command/wf/SubmitXCommand.java 
> 70b9adc1c 
>   
> core/src/main/java/org/apache/oozie/command/wf/WorkflowNotificationXCommand.java
>  62bf9b5b3 
>   core/src/main/java/org/apache/oozie/coord/CoordELFunctions.java 2761e44b0 
>   core/src/main/java/org/apache/oozie/coord/CoordUtils.java 1d97accd4 
>   core/src/main/java/org/apache/oozie/executor/jpa/BulkJPAExecutor.java 
> 42961157a 
>   
> core/src/main/java/org/apache/oozie/executor/jpa/BundleActionGetJPAExecutor.java
>  3293efd69 
>   
> core/src/main/java/org/apache/oozie/executor/jpa/BundleActionInsertJPAExecutor.java
>  4b99c1a59 
>   
> core/src/main/java/org/apache/oozie/executor/jpa/BundleActionsCountForJobGetJPAExecutor.java
>  c9f3a212d 
>   
> core/src/main/java/org/apache/oozie/executor/jpa/BundleJobGetCoordinatorsJPAExecutor.java
>  2f4665e15 
>   
> core/src/main/java/org/apache/oozie/executor/jpa/BundleJobGetForUserJPAExecutor.java
>  aef9eb79c 
>   
> core/src/main/java/org/apache/oozie/executor/jpa/BundleJobGetJPAExecutor.java 
> 7f98cd2a8 
>   
> core/src/main/java/org/apache/oozie/executor/jpa/BundleJobInfoGetJPAExecutor.java
>  6e1ea3e5b 
>   
> core/src/main/java/org/apache/oozie/executor/jpa/BundleJobInsertJPAExecutor.java
>  e7373d67d 
>   
> core/src/main/java/org/apache/oozie/executor/jpa/CoordActionGetForCheckJPAExecutor.java
>  9e8b33191 
>   
> core/src/main/java/org/apache/oozie/executor/jpa/CoordActionGetForExternalIdJPAExecutor.java
>  61614e9fa 
>   
> core/src/main/java/org/apache/oozie/executor/jpa/CoordActionGetForInfoJPAExecutor.java
>  211d594e5 
>   
> core/src/main/java/org/apache/oozie/executor/jpa/CoordActionGetForInputCheckJPAExecutor.java
>  484ff93c9 
>   
> core/src/main/java/org/apache/oozie/executor/jpa/CoordActionGetForStartJPAExecutor.java
>  14b831d65 
>   
> core/src/main/java/org/apache/oozie/executor/jpa/CoordActionGetForTimeoutJPAExecutor.java
>  c720be99d 
>   
> core/src/main/java/org/apache/oozie/executor/jpa/CoordActionGetJPAExecutor.java
>  53214ea64 
>   
> core/src/main/java/org/apache/oozie/executor/jpa/CoordActionInsertJPAExecutor.java
>  09e885793 
>   
> core/src/main/java/org/apache/oozie/executor/jpa/CoordActionRemoveJPAExecutor.java
>  8ea31a31d 
>   
> 

Re: Review Request 70964: OOZIE-3513 Migrate from Preconditions.checkNotNull and ParamChecker.notNull

2019-06-27 Thread Kinga Marton via Review Board

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




core/src/main/java/org/apache/oozie/util/ConfigUtils.java
Lines 153-155 (original), 154-156 (patched)


Please extend the error message as you did in the other cases



core/src/main/java/org/apache/oozie/util/FixedJsonInstanceSerializer.java
Lines 71-72 (original), 71-72 (patched)


Please extend the error message



core/src/main/java/org/apache/oozie/util/FixedJsonInstanceSerializer.java
Lines 77-78 (original), 77-78 (patched)


Please extend the error message



core/src/main/java/org/apache/oozie/util/FixedJsonInstanceSerializer.java
Lines 84-85 (original), 84-85 (patched)


Please extend the error message



core/src/main/java/org/apache/oozie/util/FixedJsonInstanceSerializer.java
Lines 92-94 (original), 92-94 (patched)


Please extend the error message



core/src/main/java/org/apache/oozie/util/ParamChecker.java
Lines 44-49 (original)


Until now thie method throwed IllegalArgumentException in case of null 
value. Now we will get NPE. Wouldnt cause this change issues?



core/src/main/java/org/apache/oozie/util/ParamChecker.java
Line 106 (original), 75 (patched)


In other cases you have replaces this String.format with lambdas. In this 
case is skipped it is skipped intentionally?



fluent-job/fluent-job-api/src/main/java/org/apache/oozie/fluentjob/api/mapping/ExplicitNodeConverter.java
Line 187 (original), 187 (patched)


Please extend the error message



fluent-job/fluent-job-api/src/main/java/org/apache/oozie/fluentjob/api/mapping/GraphNodesToWORKFLOWAPPConverter.java
Line 178 (original), 178 (patched)


Please extend the error message



server/src/main/java/org/apache/oozie/server/HttpConfigurationWrapper.java
Line 33 (original), 34 (patched)


please extend the error message



tools/src/main/java/org/apache/oozie/tools/OozieSharelibCLI.java
Lines 401-406 (original), 402-407 (patched)


plese add an error message



tools/src/main/java/org/apache/oozie/tools/diag/ArgParser.java
Line 106 (original), 107 (patched)


please add an error message


- Kinga Marton


On June 27, 2019, 10:38 a.m., Andras Salamon wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70964/
> ---
> 
> (Updated June 27, 2019, 10:38 a.m.)
> 
> 
> Review request for oozie and Kinga Marton.
> 
> 
> Repository: oozie-git
> 
> 
> Description
> ---
> 
> OOZIE-3513 Migrate from Preconditions.checkNotNull and ParamChecker.notNull
> 
> 
> Diffs
> -
> 
>   client/src/main/java/org/apache/oozie/client/ApiJarFactory.java bb85d6780 
>   client/src/main/java/org/apache/oozie/client/ApiJarLoader.java 6ecf485ed 
>   core/src/main/java/org/apache/oozie/BaseLocalOozieClient.java 376ea1165 
>   core/src/main/java/org/apache/oozie/XException.java fb1a381bb 
>   core/src/main/java/org/apache/oozie/action/ActionExecutorException.java 
> 9bc0e81b9 
>   core/src/main/java/org/apache/oozie/action/hadoop/GitActionExecutor.java 
> fc4d65b83 
>   core/src/main/java/org/apache/oozie/action/hadoop/HadoopTokenHelper.java 
> 6457cf508 
>   core/src/main/java/org/apache/oozie/action/hadoop/JavaActionExecutor.java 
> ec45fe495 
>   
> core/src/main/java/org/apache/oozie/action/hadoop/MapReduceActionExecutor.java
>  a509e4d98 
>   core/src/main/java/org/apache/oozie/action/hadoop/ShareLibExcluder.java 
> 9c45e5392 
>   core/src/main/java/org/apache/oozie/command/TransitionXCommand.java 
> 262a78f5b 
>   
> core/src/main/java/org/apache/oozie/command/bundle/BundleJobResumeXCommand.java
>  bc58be6ef 
>   
> core/src/main/java/org/apache/oozie/command/bundle/BundleSubmitXCommand.java 
> ab88cac6c 
>   
> core/src/main/java/org/apache/oozie/command/coord/CoordActionNotificationXCommand.java
>  d51f0d780 
>   
> core/src/main/java/org/apache/oozie/command/coord/CoordActionSkipXCommand.java
>  ef0fc2d3e 
>   
> core/src/main/java/org/apache/oozie/command/coord/CoordActionTimeOutXCommand.java
>  9646d738a 
>   
> core/src/main/java/org/apache/oozie/command/coord/CoordActionsIgnoreXCommand.java
>  88842f1f1 
> 

Re: Review Request 70762: OOZIE-3499 [Java 11] Fix TestLiteWorkflowAppParser

2019-05-31 Thread Kinga Marton via Review Board

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

(Updated May 31, 2019, 12:30 p.m.)


Review request for oozie and Andras Salamon.


Repository: oozie-git


Description
---

All the tests from TestLiteWorkflowAppParser, where global configurations are 
used are failing because with Java11 the order of the properties is not the 
same as with Java8.


Diffs (updated)
-

  
core/src/test/java/org/apache/oozie/workflow/lite/TestLiteWorkflowAppParser.java
 61e386c43 


Diff: https://reviews.apache.org/r/70762/diff/3/

Changes: https://reviews.apache.org/r/70762/diff/2-3/


Testing
---


Thanks,

Kinga Marton



Re: Review Request 70762: OOZIE-3499 [Java 11] Fix TestLiteWorkflowAppParser

2019-05-31 Thread Kinga Marton via Review Board

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

(Updated May 31, 2019, 11:28 a.m.)


Review request for oozie and Andras Salamon.


Repository: oozie-git


Description
---

All the tests from TestLiteWorkflowAppParser, where global configurations are 
used are failing because with Java11 the order of the properties is not the 
same as with Java8.


Diffs (updated)
-

  
core/src/test/java/org/apache/oozie/workflow/lite/TestLiteWorkflowAppParser.java
 61e386c43 


Diff: https://reviews.apache.org/r/70762/diff/2/

Changes: https://reviews.apache.org/r/70762/diff/1-2/


Testing
---


Thanks,

Kinga Marton



Review Request 70762: OOZIE-3499 [Java 11] Fix TestLiteWorkflowAppParser

2019-05-30 Thread Kinga Marton via Review Board

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

Review request for oozie and Andras Salamon.


Repository: oozie-git


Description
---

All the tests from TestLiteWorkflowAppParser, where global configurations are 
used are failing because with Java11 the order of the properties is not the 
same as with Java8.


Diffs
-

  
core/src/test/java/org/apache/oozie/workflow/lite/TestLiteWorkflowAppParser.java
 61e386c43 


Diff: https://reviews.apache.org/r/70762/diff/1/


Testing
---


Thanks,

Kinga Marton



Re: Review Request 70502: OOZIE-3265 properties RERUN_FAIL_NODES and RERUN_SKIP_NODES should be able to appear together

2019-04-26 Thread Kinga Marton via Review Board

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

(Updated April 26, 2019, 2:47 p.m.)


Review request for oozie and Andras Salamon.


Repository: oozie-git


Description
---

Currently when you re-run a workflow with property "oozie.wf.rerun.failnodes"  
set to true,

you can no longer re-run it again with "oozie.wf.rerun.skip.nodes" property 
specified, even if you set "oozie.wf.rerun.failnodes" to false.

This kind of limitation is not reasonable. There is only one case where 
"oozie.wf.rerun.failnodes" is true and "oozie.wf.rerun.skip.nodes" is not null 
or empty, that should be disallowed.


Diffs (updated)
-

  core/src/main/java/org/apache/oozie/DagEngine.java 70ce96122 
  core/src/test/java/org/apache/oozie/TestDagEngine.java 15f86403f 
  core/src/test/java/org/apache/oozie/command/wf/TestReRunXCommand.java 
c40b30581 
  docs/src/site/markdown/DG_WorkflowReRun.md c128681a8 


Diff: https://reviews.apache.org/r/70502/diff/2/

Changes: https://reviews.apache.org/r/70502/diff/1-2/


Testing
---

unit tests added


Thanks,

Kinga Marton



Review Request 70502: OOZIE-3265 properties RERUN_FAIL_NODES and RERUN_SKIP_NODES should be able to appear together

2019-04-18 Thread Kinga Marton via Review Board

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

Review request for oozie and Andras Salamon.


Repository: oozie-git


Description
---

Currently when you re-run a workflow with property "oozie.wf.rerun.failnodes"  
set to true,

you can no longer re-run it again with "oozie.wf.rerun.skip.nodes" property 
specified, even if you set "oozie.wf.rerun.failnodes" to false.

This kind of limitation is not reasonable. There is only one case where 
"oozie.wf.rerun.failnodes" is true and "oozie.wf.rerun.skip.nodes" is not null 
or empty, that should be disallowed.


Diffs
-

  core/src/main/java/org/apache/oozie/DagEngine.java 70ce96122 
  core/src/test/java/org/apache/oozie/TestDagEngine.java 15f86403f 


Diff: https://reviews.apache.org/r/70502/diff/1/


Testing
---

unit tests added


Thanks,

Kinga Marton



Re: Review Request 70155: OOZIE-3312 Add support for HSTS

2019-03-08 Thread Kinga Marton via Review Board

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

(Updated March 8, 2019, 1:42 p.m.)


Review request for oozie and Andras Salamon.


Repository: oozie-git


Description
---

As a security best practice we should add support for HSTS via oozie-site.xml 
in case of embedded Jetty.
https://www.owasp.org/index.php/HTTP_Strict_Transport_Security_Cheat_Sheet
http://www.eclipse.org/jetty/documentation/9.3.x/embedded-examples.html - this 
page is not available anymore

https://www.eclipse.org/jetty/documentation/9.4.15.v20190215/embedded-examples.html

 

Maybe we should even make it enabled by default when SSL is configured.


Diffs (updated)
-

  core/src/main/resources/oozie-default.xml c7f2becaa 
  docs/src/site/markdown/AG_Install.md 270b98fb0 
  server/src/main/java/org/apache/oozie/server/SSLServerConnectorFactory.java 
466cefc2e 
  
server/src/test/java/org/apache/oozie/server/TestSSLServerConnectorFactory.java 
f926a0910 


Diff: https://reviews.apache.org/r/70155/diff/3/

Changes: https://reviews.apache.org/r/70155/diff/2-3/


Testing
---

Junit + manually tested


Thanks,

Kinga Marton



Re: Review Request 70155: OOZIE-3312 Add support for HSTS

2019-03-08 Thread Kinga Marton via Review Board

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

(Updated March 8, 2019, 10:02 a.m.)


Review request for oozie and Andras Salamon.


Repository: oozie-git


Description
---

As a security best practice we should add support for HSTS via oozie-site.xml 
in case of embedded Jetty.
https://www.owasp.org/index.php/HTTP_Strict_Transport_Security_Cheat_Sheet
http://www.eclipse.org/jetty/documentation/9.3.x/embedded-examples.html - this 
page is not available anymore

https://www.eclipse.org/jetty/documentation/9.4.15.v20190215/embedded-examples.html

 

Maybe we should even make it enabled by default when SSL is configured.


Diffs (updated)
-

  core/src/main/resources/oozie-default.xml c7f2becaa 
  docs/src/site/markdown/AG_Install.md 270b98fb0 
  server/src/main/java/org/apache/oozie/server/SSLServerConnectorFactory.java 
466cefc2e 
  
server/src/test/java/org/apache/oozie/server/TestSSLServerConnectorFactory.java 
f926a0910 


Diff: https://reviews.apache.org/r/70155/diff/2/

Changes: https://reviews.apache.org/r/70155/diff/1-2/


Testing
---

Junit + manually tested


Thanks,

Kinga Marton



Re: Review Request 70072: Reduce warnings thrown while building

2019-03-01 Thread Kinga Marton via Review Board

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


Ship it!




Ship It!

- Kinga Marton


On March 1, 2019, 1:55 p.m., Denes Bodo wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70072/
> ---
> 
> (Updated March 1, 2019, 1:55 p.m.)
> 
> 
> Review request for oozie.
> 
> 
> Bugs: OOZIE-1702
> https://issues.apache.org/jira/browse/OOZIE-1702
> 
> 
> Repository: oozie-git
> 
> 
> Description
> ---
> 
> A lot of warnings are thrown during Oozie compilation, complaining about 
> javadoc mistakes, missing links etc among probably other severe ones. This 
> clutters the output. This JIRA is to fix these warnings
> 
> 
> Diffs
> -
> 
>   core/src/main/java/org/apache/oozie/action/ActionExecutor.java 1770b973a 
>   
> core/src/main/java/org/apache/oozie/action/hadoop/CredentialsProviderFactory.java
>  095cdd27c 
>   core/src/main/java/org/apache/oozie/action/hadoop/DistcpActionExecutor.java 
> a64d128e3 
>   core/src/main/java/org/apache/oozie/action/hadoop/FsActionExecutor.java 
> 634d3b853 
>   core/src/main/java/org/apache/oozie/action/hadoop/FsELFunctions.java 
> 031be25b9 
>   core/src/main/java/org/apache/oozie/action/hadoop/HCatCredentialHelper.java 
> 274db7830 
>   core/src/main/java/org/apache/oozie/action/hadoop/Hive2ActionExecutor.java 
> 0480f06e3 
>   core/src/main/java/org/apache/oozie/action/hadoop/HiveActionExecutor.java 
> f07f43199 
>   core/src/main/java/org/apache/oozie/action/hadoop/JavaActionExecutor.java 
> 231b38ea8 
>   core/src/main/java/org/apache/oozie/action/hadoop/LauncherHelper.java 
> a71d93091 
>   
> core/src/main/java/org/apache/oozie/action/hadoop/MapReduceActionExecutor.java
>  a4dd13bb3 
>   core/src/main/java/org/apache/oozie/action/hadoop/OozieJobInfo.java 
> d8b1f0393 
>   core/src/main/java/org/apache/oozie/action/hadoop/PigActionExecutor.java 
> 8465acddb 
>   core/src/main/java/org/apache/oozie/action/hadoop/ShellActionExecutor.java 
> 9591cd954 
>   core/src/main/java/org/apache/oozie/action/hadoop/SparkActionExecutor.java 
> 5f399c4d1 
>   core/src/main/java/org/apache/oozie/action/hadoop/SqoopActionExecutor.java 
> ffe27e3ff 
>   core/src/main/java/org/apache/oozie/action/ssh/SshActionExecutor.java 
> 1e37e8009 
>   core/src/main/java/org/apache/oozie/command/RerunTransitionXCommand.java 
> d3d389ee5 
>   core/src/main/java/org/apache/oozie/command/ResumeTransitionXCommand.java 
> 61244ccaa 
>   core/src/main/java/org/apache/oozie/command/SuspendTransitionXCommand.java 
> 6b3a2e5cb 
>   core/src/main/java/org/apache/oozie/command/TransitionXCommand.java 
> 0c101032e 
>   core/src/main/java/org/apache/oozie/command/XCommand.java 7b2dbd560 
>   core/src/main/java/org/apache/oozie/command/coord/BulkCoordXCommand.java 
> e6a9c5438 
>   
> core/src/main/java/org/apache/oozie/command/coord/CoordActionInputCheckXCommand.java
>  b3174d2d1 
>   
> core/src/main/java/org/apache/oozie/command/coord/CoordActionsKillXCommand.java
>  83c7e2f41 
>   core/src/main/java/org/apache/oozie/command/coord/CoordCommandUtils.java 
> 4e4a569fe 
>   core/src/main/java/org/apache/oozie/command/coord/CoordJobXCommand.java 
> 6cf7c05c6 
>   
> core/src/main/java/org/apache/oozie/command/coord/CoordMaterializeTransitionXCommand.java
>  ec9ef4181 
>   core/src/main/java/org/apache/oozie/command/coord/CoordSubmitXCommand.java 
> f46f2545d 
>   core/src/main/java/org/apache/oozie/command/coord/SLAEventsXCommand.java 
> cae9591dd 
>   core/src/main/java/org/apache/oozie/command/sla/SLAJobHistoryXCommand.java 
> 0b4045afc 
>   core/src/main/java/org/apache/oozie/command/wf/ActionStartXCommand.java 
> ea61e222a 
>   core/src/main/java/org/apache/oozie/command/wf/ActionXCommand.java 
> 22075464a 
>   core/src/main/java/org/apache/oozie/command/wf/BulkWorkflowXCommand.java 
> 74dfd52be 
>   core/src/main/java/org/apache/oozie/command/wf/ReRunXCommand.java 58f203f27 
>   core/src/main/java/org/apache/oozie/command/wf/SubmitXCommand.java 
> 1509c387c 
>   core/src/main/java/org/apache/oozie/command/wf/SuspendXCommand.java 
> ef9799002 
>   core/src/main/java/org/apache/oozie/command/wf/WorkflowXCommand.java 
> 9ed4336f6 
>   core/src/main/java/org/apache/oozie/coord/CoordELEvaluator.java 3467eef34 
>   core/src/main/java/org/apache/oozie/coord/CoordELFunctions.java c3fecd87e 
>   core/src/main/java/org/apache/oozie/coord/CoordUtils.java e0c6af962 
>   core/src/main/java/org/apache/oozie/coord/HCatELFunctions.java 12c0b81ad 
>   
> core/src/main/java/org/apache/oozie/coord/input/logic/CoordInputLogicEvaluatorUtil.java
>  2306edeb9 
>   core/src/main/java/org/apache/oozie/dependency/DependencyChecker.java 
> 5e9ae7468 
>   

Re: Review Request 70072: Reduce warnings thrown while building

2019-02-28 Thread Kinga Marton via Review Board

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




core/src/main/java/org/apache/oozie/action/hadoop/JavaActionExecutor.java
Line 1820 (original), 1820 (patched)


I would try to write something about when the Exception can be thrown 
instead of its possibility.



core/src/main/java/org/apache/oozie/action/hadoop/LauncherHelper.java
Line 186 (original), 186 (patched)


I would try to write something about when the Exception can be thrown 
instead of its possibility.



core/src/main/java/org/apache/oozie/action/hadoop/LauncherHelper.java
Line 196 (original), 196 (patched)


I would try to write something about when the Exception can be thrown 
instead of its possibility.



core/src/main/java/org/apache/oozie/action/hadoop/LauncherHelper.java
Line 206 (original), 206 (patched)


I would try to write something about when the Exception can be thrown 
instead of its possibility.



core/src/main/java/org/apache/oozie/command/coord/CoordCommandUtils.java
Line 114 (original), 114 (patched)


What kind of problem?



core/src/main/java/org/apache/oozie/command/coord/CoordCommandUtils.java
Line 223 (original), 223 (patched)


what kind of problem?



core/src/main/java/org/apache/oozie/command/coord/CoordCommandUtils.java
Line 246 (original), 246 (patched)


What kind of problem?



core/src/main/java/org/apache/oozie/command/coord/CoordCommandUtils.java
Line 380 (original), 380 (patched)


what kind of problem?



core/src/main/java/org/apache/oozie/command/coord/CoordCommandUtils.java
Line 397 (original), 397 (patched)


What kind of problem?



core/src/main/java/org/apache/oozie/command/coord/CoordCommandUtils.java
Line 523 (original), 523 (patched)


What kind of problem?



core/src/main/java/org/apache/oozie/command/coord/CoordCommandUtils.java
Line 601 (original), 601 (patched)


What kind of problem?



core/src/main/java/org/apache/oozie/coord/CoordELFunctions.java
Line 500 (original), 502 (patched)


What kind of problem?



core/src/main/java/org/apache/oozie/coord/CoordELFunctions.java
Line 523 (original), 525 (patched)


What kind of problem?



core/src/main/java/org/apache/oozie/coord/CoordELFunctions.java
Line 590 (original), 592 (patched)


What kind of problem?



core/src/main/java/org/apache/oozie/coord/CoordELFunctions.java
Line 623 (original), 625 (patched)


What kind of problem?



core/src/main/java/org/apache/oozie/coord/CoordELFunctions.java
Line 955 (original), 957 (patched)


What kind of problem?



core/src/main/java/org/apache/oozie/coord/CoordELFunctions.java
Line 1001 (original), 1003 (patched)


What kind of problem?



core/src/main/java/org/apache/oozie/coord/CoordELFunctions.java
Line 1060 (original), 1062 (patched)


What kind of problem?



core/src/main/java/org/apache/oozie/servlet/V0JobsServlet.java
Line 132 (original), 132 (patched)


If it won't happen why we have it declared in the methos signiture?



core/src/main/java/org/apache/oozie/servlet/V0JobsServlet.java
Lines 147 (patched)


If it won't happen why we have it declared in the methos signiture?



core/src/main/java/org/apache/oozie/servlet/V0JobsServlet.java
Lines 161 (patched)


If it won't happen why we have it declared in the methos signiture?



core/src/main/java/org/apache/oozie/servlet/V1JobServlet.java
Line 918 (original), 917 (patched)


I would try to figure out why/when can it be thrown, instead of having only 
this kind of information.


- Kinga Marton


On Feb. 28, 2019, 1:36 p.m., Denes Bodo wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> 

Re: Review Request 69783: OOZIE-3409 - Oozie Server : Memory leak in EL evaluation

2019-02-28 Thread Kinga Marton via Review Board

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


Ship it!




Ship It!

- Kinga Marton


On Feb. 19, 2019, 4:24 p.m., Andras Salamon wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69783/
> ---
> 
> (Updated Feb. 19, 2019, 4:24 p.m.)
> 
> 
> Review request for oozie, András Piros and Kinga Marton.
> 
> 
> Repository: oozie-git
> 
> 
> Description
> ---
> 
> OOZIE-3409 - Oozie Server : Memory leak in EL evaluation
> 
> 
> Diffs
> -
> 
>   core/pom.xml b6c07d345 
>   core/src/main/java/org/apache/oozie/command/coord/CoordSubmitXCommand.java 
> c1c644459 
>   core/src/main/java/org/apache/oozie/util/ELEvaluationException.java 
> a4df3b58d 
>   core/src/main/java/org/apache/oozie/util/ELEvaluator.java 90d79779f 
>   core/src/main/java/org/apache/oozie/util/StringUtils.java 26079be93 
>   core/src/test/java/org/apache/oozie/command/wf/TestActionErrors.java 
> 519b38406 
>   core/src/test/java/org/apache/oozie/util/TestELEvaluator.java 04a869c0b 
>   core/src/test/java/org/apache/oozie/util/TestStringUtils.java 423d6be27 
>   examples/pom.xml 8900535b2 
>   pom.xml 8950bbc25 
>   src/main/assemblies/distro.xml d28be9093 
> 
> 
> Diff: https://reviews.apache.org/r/69783/diff/4/
> 
> 
> Testing
> ---
> 
> Using grind to execute unit tests.
> Using CDH cluster to create lots of dynamic workflows.
> 
> 
> Thanks,
> 
> Andras Salamon
> 
>



Re: Review Request 69988: OOZIE-3395 Findbugs is no longer maintained

2019-02-22 Thread Kinga Marton via Review Board

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

(Updated Feb. 22, 2019, 8:30 a.m.)


Review request for oozie and Andras Salamon.


Repository: oozie-git


Description
---

https://gleclaire.github.io/findbugs-maven-plugin/

Status: Since Findbugs is no longer maintained, please use Spotbugs which has a 
Maven plugin.

The plugin author recommends to migrate to Spotbugs: https://spotbugs.github.io/
It might worth to investigate this plugin.


Diffs (updated)
-

  bin/test-patch-11-findbugs-diff c884daaa3 
  client/pom.xml f0f6a1b13 
  core/pom.xml b6c07d345 
  fluent-job/fluent-job-api/findbugs-filter.xml  
  fluent-job/fluent-job-api/pom.xml f303b4583 
  fluent-job/pom.xml bb8861d59 
  pom.xml d817140f1 
  sharelib/git/pom.xml 1c17e48ba 
  sharelib/oozie/pom.xml a53d335f9 
  sharelib/spark/pom.xml 76f69034e 
  tools/pom.xml 72a6c283a 


Diff: https://reviews.apache.org/r/69988/diff/4/

Changes: https://reviews.apache.org/r/69988/diff/3-4/


Testing
---

Tested manually


Thanks,

Kinga Marton



Re: Review Request 69988: OOZIE-3395 Findbugs is no longer maintained

2019-02-20 Thread Kinga Marton via Review Board

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

(Updated Feb. 20, 2019, 5:47 p.m.)


Review request for oozie and Andras Salamon.


Repository: oozie-git


Description
---

https://gleclaire.github.io/findbugs-maven-plugin/

Status: Since Findbugs is no longer maintained, please use Spotbugs which has a 
Maven plugin.

The plugin author recommends to migrate to Spotbugs: https://spotbugs.github.io/
It might worth to investigate this plugin.


Diffs (updated)
-

  bin/test-patch-11-findbugs-diff c884daaa3 
  client/pom.xml f0f6a1b13 
  core/pom.xml b6c07d345 
  fluent-job/fluent-job-api/findbugs-filter.xml  
  fluent-job/fluent-job-api/pom.xml f303b4583 
  fluent-job/pom.xml bb8861d59 
  pom.xml d817140f1 
  sharelib/git/pom.xml 1c17e48ba 
  sharelib/oozie/pom.xml a53d335f9 
  sharelib/spark/pom.xml 76f69034e 
  tools/pom.xml 72a6c283a 


Diff: https://reviews.apache.org/r/69988/diff/3/

Changes: https://reviews.apache.org/r/69988/diff/2-3/


Testing
---

Tested manually


Thanks,

Kinga Marton



Re: Review Request 69988: OOZIE-3395 Findbugs is no longer maintained

2019-02-20 Thread Kinga Marton via Review Board


> On Feb. 15, 2019, 10:02 a.m., Andras Salamon wrote:
> > bin/test-patch-11-findbugs-diff
> > Line 1 (original), 1 (patched)
> > 
> >
> > Can you please check the script with shellcheck

I have left a few warnings, because I wasn't sure that the shellcheck was right.


> On Feb. 15, 2019, 10:02 a.m., Andras Salamon wrote:
> > bin/test-patch-11-findbugs-diff
> > Line 275 (original), 275 (patched)
> > 
> >
> > Can we change the name to spotbugs-new.xml? Or maybe it's hardwired 
> > somewhere else?

This findbugs-new.xml is generated by findbugs-diff-0.1.0-all.jar, and 
unfortunately this value is hard-coded: 
https://github.com/AndersDJohnson/findbugs-diff/search?q=findbugs-new.xml_q=findbugs-new.xml


> On Feb. 15, 2019, 10:02 a.m., Andras Salamon wrote:
> > fluent-job/fluent-job-api/pom.xml
> > Line 80 (original), 80 (patched)
> > 
> >
> > Should we keep this filename?

I have renamed it, but we should convert the content of the file into 
annotations. I will create an issue for it.


> On Feb. 15, 2019, 10:02 a.m., Andras Salamon wrote:
> > pom.xml
> > Line 1865 (original), 1871 (patched)
> > 
> >
> > Why is it called findbug? Compatibility reasons?

findbug is a general name, even if the tool behind the check is changed, what 
we are actually doing is searching for bugs, so I think that this id is a valid 
one enem if now the spotbugs is doing the job.


- Kinga


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


On Feb. 20, 2019, 2:20 p.m., Kinga Marton wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69988/
> ---
> 
> (Updated Feb. 20, 2019, 2:20 p.m.)
> 
> 
> Review request for oozie and Andras Salamon.
> 
> 
> Repository: oozie-git
> 
> 
> Description
> ---
> 
> https://gleclaire.github.io/findbugs-maven-plugin/
> 
> Status: Since Findbugs is no longer maintained, please use Spotbugs which has 
> a Maven plugin.
> 
> The plugin author recommends to migrate to Spotbugs: 
> https://spotbugs.github.io/
> It might worth to investigate this plugin.
> 
> 
> Diffs
> -
> 
>   bin/test-patch-11-findbugs-diff c884daaa3 
>   client/pom.xml f0f6a1b13 
>   core/pom.xml b6c07d345 
>   fluent-job/fluent-job-api/findbugs-filter.xml  
>   fluent-job/fluent-job-api/pom.xml f303b4583 
>   fluent-job/pom.xml bb8861d59 
>   pom.xml d817140f1 
>   sharelib/git/pom.xml 1c17e48ba 
>   sharelib/oozie/pom.xml a53d335f9 
>   sharelib/spark/pom.xml 76f69034e 
>   tools/pom.xml 72a6c283a 
> 
> 
> Diff: https://reviews.apache.org/r/69988/diff/2/
> 
> 
> Testing
> ---
> 
> Tested manually
> 
> 
> Thanks,
> 
> Kinga Marton
> 
>



Re: Review Request 69988: OOZIE-3395 Findbugs is no longer maintained

2019-02-20 Thread Kinga Marton via Review Board

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

(Updated Feb. 20, 2019, 2:20 p.m.)


Review request for oozie and Andras Salamon.


Repository: oozie-git


Description
---

https://gleclaire.github.io/findbugs-maven-plugin/

Status: Since Findbugs is no longer maintained, please use Spotbugs which has a 
Maven plugin.

The plugin author recommends to migrate to Spotbugs: https://spotbugs.github.io/
It might worth to investigate this plugin.


Diffs (updated)
-

  bin/test-patch-11-findbugs-diff c884daaa3 
  client/pom.xml f0f6a1b13 
  core/pom.xml b6c07d345 
  fluent-job/fluent-job-api/findbugs-filter.xml  
  fluent-job/fluent-job-api/pom.xml f303b4583 
  fluent-job/pom.xml bb8861d59 
  pom.xml d817140f1 
  sharelib/git/pom.xml 1c17e48ba 
  sharelib/oozie/pom.xml a53d335f9 
  sharelib/spark/pom.xml 76f69034e 
  tools/pom.xml 72a6c283a 


Diff: https://reviews.apache.org/r/69988/diff/2/

Changes: https://reviews.apache.org/r/69988/diff/1-2/


Testing
---

Tested manually


Thanks,

Kinga Marton



Re: Review Request 69783: OOZIE-3409 - Oozie Server : Memory leak in EL evaluation

2019-02-11 Thread Kinga Marton via Review Board

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




core/src/main/java/org/apache/oozie/command/coord/CoordSubmitXCommand.java
Lines 450-461 (original)


Before the patch in case of an Exception it was converted to a 
CoordinatorJobException, but now, this conversion is missing. This can lead to 
some kind of functional changes, what we should avoid. No?



core/src/main/java/org/apache/oozie/util/ELEvaluator.java
Lines 148 (patched)


org.apache.jasper.el.ExpressionEvaluatorImpl is deprecated. Does it have a 
replacement? If yes, let's use the replacement instead of the deprecated class.



core/src/main/java/org/apache/oozie/util/ELEvaluator.java
Lines 205-209 (original), 210-214 (patched)


As I can see the evaluator.evaluate can throw 
javax.servlet.jsp.el.ELException, but in the catch part you are checking if the 
thrown exception is javax.el.ELException. Is this a bug or I am missing 
something?



core/src/main/java/org/apache/oozie/util/ELEvaluator.java
Lines 231-234 (original)


org.apache.jasper.el.ExpressionEvaluatorImpl has as well a parseExpression 
method. That method is not usable here instead of string processing?



core/src/main/java/org/apache/oozie/util/StringUtils.java
Lines 62 (patched)


Is not possible to use org.apache.jasper.el.ExpressionEvaluatorImpl's 
parseExpression method instead of this String processing?



core/src/main/java/org/apache/oozie/util/StringUtils.java
Lines 66 (patched)


before entering the while, we can return false if the expr doesn't contain 
the sequence at all.



core/src/main/java/org/apache/oozie/util/StringUtils.java
Lines 91 (patched)


Shouldn't we handle somehow the not valid expressions as well?



core/src/test/java/org/apache/oozie/util/TestStringUtils.java
Lines 73 (patched)


Before the fix this case would have thrown an Exception.


- Kinga Marton


On Jan. 28, 2019, 7:32 a.m., Andras Salamon wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69783/
> ---
> 
> (Updated Jan. 28, 2019, 7:32 a.m.)
> 
> 
> Review request for oozie, András Piros and Kinga Marton.
> 
> 
> Repository: oozie-git
> 
> 
> Description
> ---
> 
> OOZIE-3409 - Oozie Server : Memory leak in EL evaluation
> 
> 
> Diffs
> -
> 
>   core/pom.xml b6c07d345 
>   core/src/main/java/org/apache/oozie/command/coord/CoordSubmitXCommand.java 
> c1c644459 
>   core/src/main/java/org/apache/oozie/util/ELEvaluator.java 90d79779f 
>   core/src/main/java/org/apache/oozie/util/StringUtils.java 26079be93 
>   core/src/test/java/org/apache/oozie/command/wf/TestActionErrors.java 
> 519b38406 
>   core/src/test/java/org/apache/oozie/util/TestELEvaluator.java 04a869c0b 
>   core/src/test/java/org/apache/oozie/util/TestStringUtils.java 423d6be27 
>   examples/pom.xml 8900535b2 
>   pom.xml 93fffc791 
> 
> 
> Diff: https://reviews.apache.org/r/69783/diff/2/
> 
> 
> Testing
> ---
> 
> Using grind to execute unit tests.
> Using CDH cluster to create lots of dynamic workflows.
> 
> 
> Thanks,
> 
> Andras Salamon
> 
>



Re: Review Request 69668: OOZIE 2949 - Escape quotes whitespaces in Sqoop field

2019-01-31 Thread Kinga Marton via Review Board

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


Ship it!




Ship It!

- Kinga Marton


On Jan. 30, 2019, 2:45 p.m., Andras Salamon wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69668/
> ---
> 
> (Updated Jan. 30, 2019, 2:45 p.m.)
> 
> 
> Review request for oozie, András Piros, Denes Bodo, Peter Cseh, and Kinga 
> Marton.
> 
> 
> Repository: oozie-git
> 
> 
> Description
> ---
> 
> OOZIE 2949 - Escape quotes whitespaces in Sqoop  field
> 
> 
> Diffs
> -
> 
>   core/src/main/java/org/apache/oozie/ErrorCode.java e274b9d70 
>   core/src/main/java/org/apache/oozie/action/hadoop/ShellSplitter.java 
> PRE-CREATION 
>   
> core/src/main/java/org/apache/oozie/action/hadoop/ShellSplitterException.java 
> PRE-CREATION 
>   core/src/main/java/org/apache/oozie/action/hadoop/SqoopActionExecutor.java 
> 556f2cfd1 
>   core/src/main/resources/oozie-default.xml 6c7fc9ddc 
>   core/src/test/java/org/apache/oozie/action/hadoop/TestShellSplitter.java 
> PRE-CREATION 
>   docs/src/site/markdown/DG_SqoopActionExtension.md b186c5ab7 
>   
> sharelib/sqoop/src/test/java/org/apache/oozie/action/hadoop/TestSqoopActionExecutor.java
>  edfe0c739 
> 
> 
> Diff: https://reviews.apache.org/r/69668/diff/4/
> 
> 
> Testing
> ---
> 
> Unit tests locally
> 
> 
> Thanks,
> 
> Andras Salamon
> 
>



Re: Review Request 69668: OOZIE 2949 - Escape quotes whitespaces in Sqoop field

2019-01-29 Thread Kinga Marton via Review Board

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




core/src/main/java/org/apache/oozie/action/hadoop/SqoopActionExecutor.java
Lines 122 (patched)


I think using  actionConf.getBoolean(OOZIE_ACTION_SQOOP_SHELLSPLITTER, 
Boolean.FALSE) would be nicer



docs/src/site/markdown/DG_SqoopActionExtension.md
Lines 97 (patched)


"By default shell splitter splits the command on every space" - reading 
this sentence, for me is not obvious that this is the default when 
oozie.action.sqoop.shellsplitter is swiched on, or this is the general default. 

It would be good to make it more clear.



sharelib/sqoop/src/test/java/org/apache/oozie/action/hadoop/TestSqoopActionExecutor.java
Lines 115 (patched)


This constant is already defined in SqoopActionExecutor. Please make it 
package private and reuse it.


- Kinga Marton


On Jan. 22, 2019, 10:45 a.m., Andras Salamon wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69668/
> ---
> 
> (Updated Jan. 22, 2019, 10:45 a.m.)
> 
> 
> Review request for oozie, András Piros, Denes Bodo, Peter Cseh, and Kinga 
> Marton.
> 
> 
> Repository: oozie-git
> 
> 
> Description
> ---
> 
> OOZIE 2949 - Escape quotes whitespaces in Sqoop  field
> 
> 
> Diffs
> -
> 
>   core/src/main/java/org/apache/oozie/ErrorCode.java e274b9d70 
>   core/src/main/java/org/apache/oozie/action/hadoop/ShellSplitter.java 
> PRE-CREATION 
>   
> core/src/main/java/org/apache/oozie/action/hadoop/ShellSplitterException.java 
> PRE-CREATION 
>   core/src/main/java/org/apache/oozie/action/hadoop/SqoopActionExecutor.java 
> 556f2cfd1 
>   core/src/main/resources/oozie-default.xml 6c7fc9ddc 
>   core/src/test/java/org/apache/oozie/action/hadoop/TestShellSplitter.java 
> PRE-CREATION 
>   docs/src/site/markdown/DG_SqoopActionExtension.md b186c5ab7 
>   
> sharelib/sqoop/src/test/java/org/apache/oozie/action/hadoop/TestSqoopActionExecutor.java
>  edfe0c739 
> 
> 
> Diff: https://reviews.apache.org/r/69668/diff/3/
> 
> 
> Testing
> ---
> 
> Unit tests locally
> 
> 
> Thanks,
> 
> Andras Salamon
> 
>



Re: Review Request 69829: OOZIE 3243 - Flaky test TestCoordActionsKillXCommand#testActionKillCommandDate

2019-01-24 Thread Kinga Marton via Review Board

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


Ship it!




Ship It!

- Kinga Marton


On Jan. 24, 2019, 12:46 p.m., Andras Salamon wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69829/
> ---
> 
> (Updated Jan. 24, 2019, 12:46 p.m.)
> 
> 
> Review request for oozie, Kinga Marton and Peter Bacsko.
> 
> 
> Repository: oozie-git
> 
> 
> Description
> ---
> 
> OOZIE 3243 - Flaky test TestCoordActionsKillXCommand#testActionKillCommandDate
> 
> 
> Diffs
> -
> 
>   
> core/src/test/java/org/apache/oozie/command/coord/TestCoordActionsKillXCommand.java
>  c9b2f28ee 
> 
> 
> Diff: https://reviews.apache.org/r/69829/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Andras Salamon
> 
>



Re: Review Request 69771: OOZIE-3417 [FS Action] Optimize code about the command about chose fs action

2019-01-17 Thread Kinga Marton via Review Board

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

(Updated Jan. 17, 2019, 8:03 a.m.)


Review request for oozie and Andras Salamon.


Repository: oozie-git


Description
---

When I read FsActionExecutor.java, I found a not good code in this class.  When 
judging which logic to use based on commands, should use "switch/case" replace 
"if/else":
“if/else” make the code hard to read
“if/else” make the code hard to extend
“if/else” has low  efficience
So I suggest using “switch/case” instead.


Diffs (updated)
-

  core/src/main/java/org/apache/oozie/action/hadoop/FsActionExecutor.java 
de55793c 


Diff: https://reviews.apache.org/r/69771/diff/2/

Changes: https://reviews.apache.org/r/69771/diff/1-2/


Testing
---


Thanks,

Kinga Marton



Review Request 69771: OOZIE-3417 [FS Action] Optimize code about the command about chose fs action

2019-01-16 Thread Kinga Marton via Review Board

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

Review request for oozie and Andras Salamon.


Repository: oozie-git


Description
---

When I read FsActionExecutor.java, I found a not good code in this class.  When 
judging which logic to use based on commands, should use "switch/case" replace 
"if/else":
“if/else” make the code hard to read
“if/else” make the code hard to extend
“if/else” has low  efficience
So I suggest using “switch/case” instead.


Diffs
-

  core/src/main/java/org/apache/oozie/action/hadoop/FsActionExecutor.java 
de55793c 


Diff: https://reviews.apache.org/r/69771/diff/1/


Testing
---


Thanks,

Kinga Marton



Re: Review Request 69668: OOZIE 2949 - Escape quotes whitespaces in Sqoop field

2019-01-07 Thread Kinga Marton via Review Board

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




core/src/main/java/org/apache/oozie/action/hadoop/SqoopActionExecutor.java
Lines 258-273 (patched)


For better readability I would split this in smaller well named methods 
like: checkAndHandleEscapig(), checkAndHandleQuoting()



core/src/test/java/org/apache/oozie/action/hadoop/TestShellSplitter.java
Lines 68 (patched)


Let't try to get some more real life examples as well, and if it is the 
case expand the tests.
Also I would add some edge cases: starting/ending with whitespace, mixed 
quotation marks, unclosed quotation, trailing escape characters, etc.



sharelib/sqoop/src/test/java/org/apache/oozie/action/hadoop/TestSqoopActionExecutor.java
Lines 242-244 (patched)


Please remove commented code


- Kinga Marton


On Jan. 4, 2019, 3:48 p.m., Andras Salamon wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69668/
> ---
> 
> (Updated Jan. 4, 2019, 3:48 p.m.)
> 
> 
> Review request for oozie, András Piros, Denes Bodo, Peter Cseh, and Kinga 
> Marton.
> 
> 
> Repository: oozie-git
> 
> 
> Description
> ---
> 
> OOZIE 2949 - Escape quotes whitespaces in Sqoop  field
> 
> 
> Diffs
> -
> 
>   core/src/main/java/org/apache/oozie/action/hadoop/SqoopActionExecutor.java 
> 556f2cfd1 
>   core/src/test/java/org/apache/oozie/action/hadoop/TestShellSplitter.java 
> PRE-CREATION 
>   
> sharelib/sqoop/src/test/java/org/apache/oozie/action/hadoop/TestSqoopActionExecutor.java
>  edfe0c739 
> 
> 
> Diff: https://reviews.apache.org/r/69668/diff/1/
> 
> 
> Testing
> ---
> 
> Unit tests locally
> 
> 
> Thanks,
> 
> Andras Salamon
> 
>



Re: Review Request 69492: OOZIE-3389 Getting input dependency list on the UI throws NPE

2018-12-01 Thread Kinga Marton via Review Board

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


Ship it!




Ship It!

- Kinga Marton


On Dec. 1, 2018, 2:21 p.m., András Piros wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69492/
> ---
> 
> (Updated Dec. 1, 2018, 2:21 p.m.)
> 
> 
> Review request for oozie, Andras Salamon and Kinga Marton.
> 
> 
> Repository: oozie-git
> 
> 
> Description
> ---
> 
> OOZIE-3389 Getting input dependency list on the UI throws NPE
> 
> 
> Diffs
> -
> 
>   
> core/src/main/java/org/apache/oozie/command/coord/CoordActionMissingDependenciesXCommand.java
>  d37cfe5125a5c3a665b3e33e6ac3fd5581fe278f 
>   
> core/src/main/java/org/apache/oozie/coord/input/dependency/CoordOldInputDependency.java
>  56aef1c1779ed7446852af42f25c5ca058e473e4 
>   
> core/src/test/java/org/apache/oozie/coord/input/dependency/TestCoordOldInputDependency.java
>  PRE-CREATION 
>   core/src/test/java/org/apache/oozie/servlet/TestV2JobServlet.java 
> bacfe89bb3d836da1f6e559fa68a5bf4db970b09 
> 
> 
> Diff: https://reviews.apache.org/r/69492/diff/2/
> 
> 
> Testing
> ---
> 
> Introduced new unit test class `TestCoordOldInputDependency`.
> 
> 
> Thanks,
> 
> András Piros
> 
>



Re: Review Request 69437: OOZIE-3384 - [tests] TestWorkflowActionRetryInfoXCommand#testRetryConsoleUrlForked() is flaky

2018-11-28 Thread Kinga Marton via Review Board

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


Ship it!




Ship It!

- Kinga Marton


On Nov. 28, 2018, 9:03 a.m., Andras Salamon wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69437/
> ---
> 
> (Updated Nov. 28, 2018, 9:03 a.m.)
> 
> 
> Review request for oozie, András Piros and Kinga Marton.
> 
> 
> Repository: oozie-git
> 
> 
> Description
> ---
> 
> OOZIE-3384 - [tests] 
> TestWorkflowActionRetryInfoXCommand#testRetryConsoleUrlForked() is flaky
> 
> 
> Diffs
> -
> 
>   
> core/src/test/java/org/apache/oozie/command/wf/TestWorkflowActionRetryInfoXCommand.java
>  5c06ae554 
> 
> 
> Diff: https://reviews.apache.org/r/69437/diff/3/
> 
> 
> Testing
> ---
> 
> Executed 50 times using grind: 
> 
> 50/50 tests complete
> 
> 
> Thanks,
> 
> Andras Salamon
> 
>



Re: Review Request 69437: OOZIE-3384 - [tests] TestWorkflowActionRetryInfoXCommand#testRetryConsoleUrlForked() is flaky

2018-11-28 Thread Kinga Marton via Review Board


> On Nov. 28, 2018, 8:28 a.m., Kinga Marton wrote:
> > core/src/test/java/org/apache/oozie/command/wf/TestWorkflowActionRetryInfoXCommand.java
> > Lines 22 (patched)
> > 
> >
> > nit: unused iport

*import


- Kinga


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


On Nov. 26, 2018, 11:17 a.m., Andras Salamon wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69437/
> ---
> 
> (Updated Nov. 26, 2018, 11:17 a.m.)
> 
> 
> Review request for oozie, András Piros and Kinga Marton.
> 
> 
> Repository: oozie-git
> 
> 
> Description
> ---
> 
> OOZIE-3384 - [tests] 
> TestWorkflowActionRetryInfoXCommand#testRetryConsoleUrlForked() is flaky
> 
> 
> Diffs
> -
> 
>   
> core/src/test/java/org/apache/oozie/command/wf/TestWorkflowActionRetryInfoXCommand.java
>  5c06ae554 
> 
> 
> Diff: https://reviews.apache.org/r/69437/diff/2/
> 
> 
> Testing
> ---
> 
> Executed 50 times using grind: 
> 
> 50/50 tests complete
> 
> 
> Thanks,
> 
> Andras Salamon
> 
>



Re: Review Request 69437: OOZIE-3384 - [tests] TestWorkflowActionRetryInfoXCommand#testRetryConsoleUrlForked() is flaky

2018-11-28 Thread Kinga Marton via Review Board

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




core/src/test/java/org/apache/oozie/command/wf/TestWorkflowActionRetryInfoXCommand.java
Lines 22 (patched)


nit: unused iport


- Kinga Marton


On Nov. 26, 2018, 11:17 a.m., Andras Salamon wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69437/
> ---
> 
> (Updated Nov. 26, 2018, 11:17 a.m.)
> 
> 
> Review request for oozie, András Piros and Kinga Marton.
> 
> 
> Repository: oozie-git
> 
> 
> Description
> ---
> 
> OOZIE-3384 - [tests] 
> TestWorkflowActionRetryInfoXCommand#testRetryConsoleUrlForked() is flaky
> 
> 
> Diffs
> -
> 
>   
> core/src/test/java/org/apache/oozie/command/wf/TestWorkflowActionRetryInfoXCommand.java
>  5c06ae554 
> 
> 
> Diff: https://reviews.apache.org/r/69437/diff/2/
> 
> 
> Testing
> ---
> 
> Executed 50 times using grind: 
> 
> 50/50 tests complete
> 
> 
> Thanks,
> 
> Andras Salamon
> 
>



Re: Review Request 69348: OOZIE-3381 [coordinator] Enhance logging of CoordElFunctions

2018-11-22 Thread Kinga Marton via Review Board

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


Ship it!




Ship It!

- Kinga Marton


On Nov. 20, 2018, 10:54 a.m., András Piros wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69348/
> ---
> 
> (Updated Nov. 20, 2018, 10:54 a.m.)
> 
> 
> Review request for oozie and Kinga Marton.
> 
> 
> Repository: oozie-git
> 
> 
> Description
> ---
> 
> OOZIE-3381 [coordinator] Enhance logging of CoordElFunctions
> 
> 
> Diffs
> -
> 
>   core/src/main/java/org/apache/oozie/command/XCommand.java 
> a80444e14134ec4234e6edfef0888cf1e76566df 
>   core/src/main/java/org/apache/oozie/coord/CoordELFunctions.java 
> 10f4f0d8d14fbb8d60a2d09a45b8f3b3b089f461 
>   core/src/test/java/org/apache/oozie/command/coord/CoordELExtensions.java 
> 796d19cfc9cbd03211746f9b94d0f67371249c7e 
>   core/src/test/java/org/apache/oozie/coord/TestOozieTimeUnitConverter.java 
> PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/69348/diff/2/
> 
> 
> Testing
> ---
> 
> `TestCoordELFunctions`
> 
> 
> Thanks,
> 
> András Piros
> 
>



Re: Review Request 69348: OOZIE-3381 [coordinator] Enhance logging of CoordElFunctions

2018-11-15 Thread Kinga Marton via Review Board

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




core/src/main/java/org/apache/oozie/coord/CoordELFunctions.java
Lines 1587 (patched)


Services.getConf() is depricated. Can you please change it to 
ConfigurationService.getBoolean()?



core/src/main/java/org/apache/oozie/coord/CoordELFunctions.java
Lines 1644 (patched)


I know this is an old code, but why do we need an array here? Everywhere is 
used only the first element from it. Can't we change it to use a simple int 
instead of an array of ints (maybe in a follow up issue)?



core/src/main/java/org/apache/oozie/coord/CoordELFunctions.java
Lines 1672 (patched)


I think that this is unnecessary here because instCount is already 
instantiated (see line 1644)



core/src/main/java/org/apache/oozie/coord/CoordELFunctions.java
Lines 1781 (patched)


Please remove comment



core/src/main/java/org/apache/oozie/coord/CoordELFunctions.java
Lines 1784 (patched)


Please remove comment



core/src/main/java/org/apache/oozie/coord/CoordELFunctions.java
Lines 1883 (patched)


Can you please add some Unit tests for this method?


- Kinga Marton


On Nov. 15, 2018, 11:47 a.m., András Piros wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69348/
> ---
> 
> (Updated Nov. 15, 2018, 11:47 a.m.)
> 
> 
> Review request for oozie and Kinga Marton.
> 
> 
> Repository: oozie-git
> 
> 
> Description
> ---
> 
> OOZIE-3381 [coordinator] Enhance logging of CoordElFunctions
> 
> 
> Diffs
> -
> 
>   core/src/main/java/org/apache/oozie/command/XCommand.java 
> a80444e14134ec4234e6edfef0888cf1e76566df 
>   core/src/main/java/org/apache/oozie/coord/CoordELFunctions.java 
> 10f4f0d8d14fbb8d60a2d09a45b8f3b3b089f461 
> 
> 
> Diff: https://reviews.apache.org/r/69348/diff/1/
> 
> 
> Testing
> ---
> 
> `TestCoordELFunctions`
> 
> 
> Thanks,
> 
> András Piros
> 
>



Re: Review Request 69298: OOZIE-3380 TestCoordMaterializeTransitionXCommand failure after DST change date

2018-11-12 Thread Kinga Marton via Review Board

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


Ship it!




Ship It!

- Kinga Marton


On Nov. 12, 2018, 2:03 p.m., Andras Salamon wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69298/
> ---
> 
> (Updated Nov. 12, 2018, 2:03 p.m.)
> 
> 
> Review request for oozie, András Piros and Kinga Marton.
> 
> 
> Repository: oozie-git
> 
> 
> Description
> ---
> 
> OOZIE-3380 TestCoordMaterializeTransitionXCommand failure after DST change 
> date
> 
> 
> Diffs
> -
> 
>   
> core/src/test/java/org/apache/oozie/command/coord/TestCoordMaterializeTransitionXCommand.java
>  49b8732ad 
> 
> 
> Diff: https://reviews.apache.org/r/69298/diff/2/
> 
> 
> Testing
> ---
> 
> Execued unit tests during the 3 day period after the DST change and also 
> after more than three days.
> 
> 
> Thanks,
> 
> Andras Salamon
> 
>



Re: Review Request 69298: OOZIE-3380 TestCoordMaterializeTransitionXCommand failure after DST change date

2018-11-12 Thread Kinga Marton via Review Board

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




core/src/test/java/org/apache/oozie/command/coord/TestCoordMaterializeTransitionXCommand.java
Lines 64-66 (patched)


Can you please add static final modifiers for the constants?



core/src/test/java/org/apache/oozie/command/coord/TestCoordMaterializeTransitionXCommand.java
Lines 774 (patched)


This issue will affect only one direction of DST change? (the end of DST)


- Kinga Marton


On Nov. 8, 2018, 9:23 a.m., Andras Salamon wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69298/
> ---
> 
> (Updated Nov. 8, 2018, 9:23 a.m.)
> 
> 
> Review request for oozie, András Piros and Kinga Marton.
> 
> 
> Repository: oozie-git
> 
> 
> Description
> ---
> 
> OOZIE-3380 TestCoordMaterializeTransitionXCommand failure after DST change 
> date
> 
> 
> Diffs
> -
> 
>   
> core/src/test/java/org/apache/oozie/command/coord/TestCoordMaterializeTransitionXCommand.java
>  49b8732ad 
> 
> 
> Diff: https://reviews.apache.org/r/69298/diff/1/
> 
> 
> Testing
> ---
> 
> Execued unit tests during the 3 day period after the DST change and also 
> after more than three days.
> 
> 
> Thanks,
> 
> Andras Salamon
> 
>



Re: Review Request 68628: OOZIE-2734 - Switch docs from twiki to markdown

2018-09-13 Thread Kinga Marton via Review Board

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




docs/src/site/twiki/DG_CommandLineTool.twiki
Line 265 (original), 277 (patched)


html: (CLASS=)



docs/src/site/twiki/DG_CommandLineTool.twiki
Line 753 (original), 790 (patched)


html (opt1`val1;opt2`val1)



docs/src/site/twiki/DG_CommandLineTool.twiki
Lines 1048-1050 (original), 1096-1098 (patched)


html (listing)


- Kinga Marton


On Sept. 12, 2018, 8:50 a.m., Andras Salamon wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68628/
> ---
> 
> (Updated Sept. 12, 2018, 8:50 a.m.)
> 
> 
> Review request for oozie and András Piros.
> 
> 
> Repository: oozie-git
> 
> 
> Description
> ---
> 
> OOZIE-2734 - Switch docs from twiki to markdown
> 
> 
> Diffs
> -
> 
>   README.txt 8cc0f59ef 
>   docs/pom.xml ae2e94c9c 
>   docs/src/site/markdown/AG_OozieLogging.md PRE-CREATION 
>   docs/src/site/markdown/DG_QuickStart.md PRE-CREATION 
>   docs/src/site/markdown/ENG_Building.md PRE-CREATION 
>   docs/src/site/markdown/ENG_Custom_Authentication.md PRE-CREATION 
>   docs/src/site/markdown/index.md PRE-CREATION 
>   docs/src/site/twiki/AG_ActionConfiguration.twiki 8c032a754 
>   docs/src/site/twiki/AG_HadoopConfiguration.twiki 528bf4a41 
>   docs/src/site/twiki/AG_Install.twiki b8031c82e 
>   docs/src/site/twiki/AG_Monitoring.twiki 425f55435 
>   docs/src/site/twiki/AG_OozieLogging.twiki ecdcfd33e 
>   docs/src/site/twiki/AG_OozieUpgrade.twiki 302406482 
>   docs/src/site/twiki/BundleFunctionalSpec.twiki 9749df597 
>   docs/src/site/twiki/CoordinatorFunctionalSpec.twiki cd416d4cd 
>   docs/src/site/twiki/DG_ActionAuthentication.twiki bbf2d57d3 
>   docs/src/site/twiki/DG_CommandLineTool.twiki 8d33c508e 
>   docs/src/site/twiki/DG_CoordinatorRerun.twiki fbb1376d5 
>   docs/src/site/twiki/DG_CustomActionExecutor.twiki 783148495 
>   docs/src/site/twiki/DG_DistCpActionExtension.twiki 8bab3da44 
>   docs/src/site/twiki/DG_EmailActionExtension.twiki 4de290ccf 
>   docs/src/site/twiki/DG_Examples.twiki 5323a1773 
>   docs/src/site/twiki/DG_FluentJobAPI.twiki c8b764bb0 
>   docs/src/site/twiki/DG_HCatalogIntegration.twiki d3107b453 
>   docs/src/site/twiki/DG_Hive2ActionExtension.twiki efbe56dab 
>   docs/src/site/twiki/DG_HiveActionExtension.twiki aaa74fa7b 
>   docs/src/site/twiki/DG_JMSNotifications.twiki a4b0f0d1a 
>   docs/src/site/twiki/DG_Overview.twiki 3ec94a2f1 
>   docs/src/site/twiki/DG_QuickStart.twiki d6a0069ac 
>   docs/src/site/twiki/DG_SLAMonitoring.twiki c91c227dc 
>   docs/src/site/twiki/DG_ShellActionExtension.twiki 5894c280a 
>   docs/src/site/twiki/DG_SparkActionExtension.twiki ce80e45fc 
>   docs/src/site/twiki/DG_SqoopActionExtension.twiki 0317d0cec 
>   docs/src/site/twiki/DG_SshActionExtension.twiki 5a51d49c5 
>   docs/src/site/twiki/DG_WorkflowReRun.twiki 88d982b75 
>   docs/src/site/twiki/ENG_Building.twiki b86102683 
>   docs/src/site/twiki/ENG_Custom_Authentication.twiki 3b8202d0d 
>   docs/src/site/twiki/ENG_MiniOozie.twiki 0b1628986 
>   docs/src/site/twiki/WebServicesAPI.twiki f9008a60b 
>   docs/src/site/twiki/WorkflowFunctionalSpec.twiki 46a454c3a 
>   docs/src/site/twiki/index.twiki 3003fa992 
>   pom.xml 423d19d97 
> 
> 
> Diff: https://reviews.apache.org/r/68628/diff/2/
> 
> 
> Testing
> ---
> 
> HTML generated
> 
> 
> Thanks,
> 
> Andras Salamon
> 
>



Re: Review Request 68628: OOZIE-2734 - Switch docs from twiki to markdown

2018-09-12 Thread Kinga Marton via Review Board

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




docs/src/site/twiki/AG_Install.twiki
Line 768 (original), 797 (patched)


html: a newline is missing



docs/src/site/twiki/AG_Install.twiki
Lines 844-846 (original), 877-879 (patched)


html



docs/src/site/twiki/AG_Install.twiki
Lines 936-937 (original), 976-977 (patched)


html: I think we should remove the link from this 2 places and just 
highlight them



docs/src/site/twiki/BundleFunctionalSpec.twiki
Line 266 (original), 270 (patched)


Missing numbering. The previous headers from the same level has numbering.



docs/src/site/twiki/BundleFunctionalSpec.twiki
Lines 274 (patched)


Missing numbering. The previous headers from the same level has numbering.



docs/src/site/twiki/CoordinatorFunctionalSpec.twiki
Line 210 (original), 225 (patched)


html (strange table header) - is valid for the whole file



docs/src/site/twiki/CoordinatorFunctionalSpec.twiki
Line 576 (original), 602 (patched)


html: I think that here we need the '=' signs



docs/src/site/twiki/CoordinatorFunctionalSpec.twiki
Line 2543 (original), 2645 (patched)


html (I think that there is an unnecessary space between * and silently...)



docs/src/site/twiki/CoordinatorFunctionalSpec.twiki
Line 2706 (original), 2813 (patched)


html (the last '=' is not converted to '`')



docs/src/site/twiki/CoordinatorFunctionalSpec.twiki
Line 3407 (original), 3519 (patched)


html (highlight)


- Kinga Marton


On Sept. 12, 2018, 8:50 a.m., Andras Salamon wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68628/
> ---
> 
> (Updated Sept. 12, 2018, 8:50 a.m.)
> 
> 
> Review request for oozie and András Piros.
> 
> 
> Repository: oozie-git
> 
> 
> Description
> ---
> 
> OOZIE-2734 - Switch docs from twiki to markdown
> 
> 
> Diffs
> -
> 
>   README.txt 8cc0f59ef 
>   docs/pom.xml ae2e94c9c 
>   docs/src/site/markdown/AG_OozieLogging.md PRE-CREATION 
>   docs/src/site/markdown/DG_QuickStart.md PRE-CREATION 
>   docs/src/site/markdown/ENG_Building.md PRE-CREATION 
>   docs/src/site/markdown/ENG_Custom_Authentication.md PRE-CREATION 
>   docs/src/site/markdown/index.md PRE-CREATION 
>   docs/src/site/twiki/AG_ActionConfiguration.twiki 8c032a754 
>   docs/src/site/twiki/AG_HadoopConfiguration.twiki 528bf4a41 
>   docs/src/site/twiki/AG_Install.twiki b8031c82e 
>   docs/src/site/twiki/AG_Monitoring.twiki 425f55435 
>   docs/src/site/twiki/AG_OozieLogging.twiki ecdcfd33e 
>   docs/src/site/twiki/AG_OozieUpgrade.twiki 302406482 
>   docs/src/site/twiki/BundleFunctionalSpec.twiki 9749df597 
>   docs/src/site/twiki/CoordinatorFunctionalSpec.twiki cd416d4cd 
>   docs/src/site/twiki/DG_ActionAuthentication.twiki bbf2d57d3 
>   docs/src/site/twiki/DG_CommandLineTool.twiki 8d33c508e 
>   docs/src/site/twiki/DG_CoordinatorRerun.twiki fbb1376d5 
>   docs/src/site/twiki/DG_CustomActionExecutor.twiki 783148495 
>   docs/src/site/twiki/DG_DistCpActionExtension.twiki 8bab3da44 
>   docs/src/site/twiki/DG_EmailActionExtension.twiki 4de290ccf 
>   docs/src/site/twiki/DG_Examples.twiki 5323a1773 
>   docs/src/site/twiki/DG_FluentJobAPI.twiki c8b764bb0 
>   docs/src/site/twiki/DG_HCatalogIntegration.twiki d3107b453 
>   docs/src/site/twiki/DG_Hive2ActionExtension.twiki efbe56dab 
>   docs/src/site/twiki/DG_HiveActionExtension.twiki aaa74fa7b 
>   docs/src/site/twiki/DG_JMSNotifications.twiki a4b0f0d1a 
>   docs/src/site/twiki/DG_Overview.twiki 3ec94a2f1 
>   docs/src/site/twiki/DG_QuickStart.twiki d6a0069ac 
>   docs/src/site/twiki/DG_SLAMonitoring.twiki c91c227dc 
>   docs/src/site/twiki/DG_ShellActionExtension.twiki 5894c280a 
>   docs/src/site/twiki/DG_SparkActionExtension.twiki ce80e45fc 
>   docs/src/site/twiki/DG_SqoopActionExtension.twiki 0317d0cec 
>   docs/src/site/twiki/DG_SshActionExtension.twiki 5a51d49c5 
>   docs/src/site/twiki/DG_WorkflowReRun.twiki 88d982b75 
>   docs/src/site/twiki/ENG_Building.twiki b86102683 
>   docs/src/site/twiki/ENG_Custom_Authentication.twiki 3b8202d0d 
>   docs/src/site/twiki/ENG_MiniOozie.twiki 0b1628986 
>   docs/src/site/twiki/WebServicesAPI.twiki f9008a60b 
>   

Re: Review Request 68628: OOZIE-2734 - Switch docs from twiki to markdown

2018-09-11 Thread Kinga Marton via Review Board

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




docs/src/site/twiki/BundleFunctionalSpec.twiki
Line 39 (original), 39 (patched)


JSP 2.0 Specification (JSP.2.3), link is showing
System Error: 404



docs/src/site/twiki/CoordinatorFunctionalSpec.twiki
Line 118 (original), 133 (patched)


JSP 2.0 Specification (JSP.2.3), link is showing
System Error: 404



docs/src/site/twiki/WorkflowFunctionalSpec.twiki
Lines 1743-1744 (original), 1806-1807 (patched)


html (=String wf:conf(String name)=)



docs/src/site/twiki/WorkflowFunctionalSpec.twiki
Line 2372 (original), 2442 (patched)


Do we need here the link? (so...@mysockshost.mydomain.com)



docs/src/site/twiki/WorkflowFunctionalSpec.twiki
Line 2592 (original), 2664 (patched)


the link is not working



docs/src/site/twiki/WorkflowFunctionalSpec.twiki
Line 2595 (original), 2667 (patched)


the link is not working



docs/src/site/twiki/WorkflowFunctionalSpec.twiki
Line 2617 (original), 2689 (patched)


the link is not working


- Kinga Marton


On Sept. 5, 2018, 10 a.m., Andras Salamon wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68628/
> ---
> 
> (Updated Sept. 5, 2018, 10 a.m.)
> 
> 
> Review request for oozie and András Piros.
> 
> 
> Repository: oozie-git
> 
> 
> Description
> ---
> 
> OOZIE-2734 - Switch docs from twiki to markdown
> 
> 
> Diffs
> -
> 
>   docs/pom.xml ae2e94c9c 
>   docs/src/site/markdown/AG_OozieLogging.md PRE-CREATION 
>   docs/src/site/markdown/ENG_Building.md PRE-CREATION 
>   docs/src/site/markdown/index.md PRE-CREATION 
>   docs/src/site/twiki/AG_ActionConfiguration.twiki 8c032a754 
>   docs/src/site/twiki/AG_HadoopConfiguration.twiki 528bf4a41 
>   docs/src/site/twiki/AG_Install.twiki b8031c82e 
>   docs/src/site/twiki/AG_Monitoring.twiki 425f55435 
>   docs/src/site/twiki/AG_OozieLogging.twiki ecdcfd33e 
>   docs/src/site/twiki/AG_OozieUpgrade.twiki 302406482 
>   docs/src/site/twiki/BundleFunctionalSpec.twiki 9749df597 
>   docs/src/site/twiki/CoordinatorFunctionalSpec.twiki cd416d4cd 
>   docs/src/site/twiki/DG_ActionAuthentication.twiki bbf2d57d3 
>   docs/src/site/twiki/DG_CommandLineTool.twiki 8d33c508e 
>   docs/src/site/twiki/DG_CoordinatorRerun.twiki fbb1376d5 
>   docs/src/site/twiki/DG_CustomActionExecutor.twiki 783148495 
>   docs/src/site/twiki/DG_DistCpActionExtension.twiki 8bab3da44 
>   docs/src/site/twiki/DG_EmailActionExtension.twiki 4de290ccf 
>   docs/src/site/twiki/DG_Examples.twiki 5323a1773 
>   docs/src/site/twiki/DG_FluentJobAPI.twiki c8b764bb0 
>   docs/src/site/twiki/DG_HCatalogIntegration.twiki d3107b453 
>   docs/src/site/twiki/DG_Hive2ActionExtension.twiki efbe56dab 
>   docs/src/site/twiki/DG_HiveActionExtension.twiki aaa74fa7b 
>   docs/src/site/twiki/DG_JMSNotifications.twiki a4b0f0d1a 
>   docs/src/site/twiki/DG_Overview.twiki 3ec94a2f1 
>   docs/src/site/twiki/DG_QuickStart.twiki d6a0069ac 
>   docs/src/site/twiki/DG_SLAMonitoring.twiki c91c227dc 
>   docs/src/site/twiki/DG_ShellActionExtension.twiki 5894c280a 
>   docs/src/site/twiki/DG_SparkActionExtension.twiki ce80e45fc 
>   docs/src/site/twiki/DG_SqoopActionExtension.twiki 0317d0cec 
>   docs/src/site/twiki/DG_SshActionExtension.twiki 5a51d49c5 
>   docs/src/site/twiki/DG_WorkflowReRun.twiki 88d982b75 
>   docs/src/site/twiki/ENG_Building.twiki b86102683 
>   docs/src/site/twiki/ENG_Custom_Authentication.twiki 3b8202d0d 
>   docs/src/site/twiki/ENG_MiniOozie.twiki 0b1628986 
>   docs/src/site/twiki/WebServicesAPI.twiki f9008a60b 
>   docs/src/site/twiki/WorkflowFunctionalSpec.twiki 46a454c3a 
>   docs/src/site/twiki/index.twiki 3003fa992 
>   pom.xml 423d19d97 
> 
> 
> Diff: https://reviews.apache.org/r/68628/diff/1/
> 
> 
> Testing
> ---
> 
> HTML generated
> 
> 
> Thanks,
> 
> Andras Salamon
> 
>



Re: Review Request 68628: OOZIE-2734 - Switch docs from twiki to markdown

2018-09-11 Thread Kinga Marton via Review Board

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




docs/src/site/twiki/WebServicesAPI.twiki
Lines 583-586 (original), 612-615 (patched)


html (coloring)



docs/src/site/twiki/WebServicesAPI.twiki
Lines 672-680 (original), 705-713 (patched)


html



docs/src/site/twiki/WebServicesAPI.twiki
Lines 746-757 (original), 781-792 (patched)


html (enumeration)



docs/src/site/twiki/WebServicesAPI.twiki
Lines 846-857 (original), 884-895 (patched)


html (enumeration)



docs/src/site/twiki/WebServicesAPI.twiki
Lines 929-938 (original), 969-978 (patched)


html (enumeration)



docs/src/site/twiki/WebServicesAPI.twiki
Lines 1558-1564 (original), 1639-1645 (patched)


html



docs/src/site/twiki/WebServicesAPI.twiki
Lines 1651-1652 (original), 1742-1743 (patched)


html



docs/src/site/twiki/WebServicesAPI.twiki
Lines 1902 (patched)


html



docs/src/site/twiki/WebServicesAPI.twiki
Lines 2081 (patched)


html



docs/src/site/twiki/WebServicesAPI.twiki
Lines 2101 (patched)


html


- Kinga Marton


On Sept. 5, 2018, 10 a.m., Andras Salamon wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68628/
> ---
> 
> (Updated Sept. 5, 2018, 10 a.m.)
> 
> 
> Review request for oozie and András Piros.
> 
> 
> Repository: oozie-git
> 
> 
> Description
> ---
> 
> OOZIE-2734 - Switch docs from twiki to markdown
> 
> 
> Diffs
> -
> 
>   docs/pom.xml ae2e94c9c 
>   docs/src/site/markdown/AG_OozieLogging.md PRE-CREATION 
>   docs/src/site/markdown/ENG_Building.md PRE-CREATION 
>   docs/src/site/markdown/index.md PRE-CREATION 
>   docs/src/site/twiki/AG_ActionConfiguration.twiki 8c032a754 
>   docs/src/site/twiki/AG_HadoopConfiguration.twiki 528bf4a41 
>   docs/src/site/twiki/AG_Install.twiki b8031c82e 
>   docs/src/site/twiki/AG_Monitoring.twiki 425f55435 
>   docs/src/site/twiki/AG_OozieLogging.twiki ecdcfd33e 
>   docs/src/site/twiki/AG_OozieUpgrade.twiki 302406482 
>   docs/src/site/twiki/BundleFunctionalSpec.twiki 9749df597 
>   docs/src/site/twiki/CoordinatorFunctionalSpec.twiki cd416d4cd 
>   docs/src/site/twiki/DG_ActionAuthentication.twiki bbf2d57d3 
>   docs/src/site/twiki/DG_CommandLineTool.twiki 8d33c508e 
>   docs/src/site/twiki/DG_CoordinatorRerun.twiki fbb1376d5 
>   docs/src/site/twiki/DG_CustomActionExecutor.twiki 783148495 
>   docs/src/site/twiki/DG_DistCpActionExtension.twiki 8bab3da44 
>   docs/src/site/twiki/DG_EmailActionExtension.twiki 4de290ccf 
>   docs/src/site/twiki/DG_Examples.twiki 5323a1773 
>   docs/src/site/twiki/DG_FluentJobAPI.twiki c8b764bb0 
>   docs/src/site/twiki/DG_HCatalogIntegration.twiki d3107b453 
>   docs/src/site/twiki/DG_Hive2ActionExtension.twiki efbe56dab 
>   docs/src/site/twiki/DG_HiveActionExtension.twiki aaa74fa7b 
>   docs/src/site/twiki/DG_JMSNotifications.twiki a4b0f0d1a 
>   docs/src/site/twiki/DG_Overview.twiki 3ec94a2f1 
>   docs/src/site/twiki/DG_QuickStart.twiki d6a0069ac 
>   docs/src/site/twiki/DG_SLAMonitoring.twiki c91c227dc 
>   docs/src/site/twiki/DG_ShellActionExtension.twiki 5894c280a 
>   docs/src/site/twiki/DG_SparkActionExtension.twiki ce80e45fc 
>   docs/src/site/twiki/DG_SqoopActionExtension.twiki 0317d0cec 
>   docs/src/site/twiki/DG_SshActionExtension.twiki 5a51d49c5 
>   docs/src/site/twiki/DG_WorkflowReRun.twiki 88d982b75 
>   docs/src/site/twiki/ENG_Building.twiki b86102683 
>   docs/src/site/twiki/ENG_Custom_Authentication.twiki 3b8202d0d 
>   docs/src/site/twiki/ENG_MiniOozie.twiki 0b1628986 
>   docs/src/site/twiki/WebServicesAPI.twiki f9008a60b 
>   docs/src/site/twiki/WorkflowFunctionalSpec.twiki 46a454c3a 
>   docs/src/site/twiki/index.twiki 3003fa992 
>   pom.xml 423d19d97 
> 
> 
> Diff: https://reviews.apache.org/r/68628/diff/1/
> 
> 
> Testing
> ---
> 
> HTML generated
> 
> 
> Thanks,
> 
> Andras Salamon
> 
>



Re: Review Request 68628: OOZIE-2734 - Switch docs from twiki to markdown

2018-09-10 Thread Kinga Marton via Review Board

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



Checked pages:

ENG_MiniOozie.html
ENG_Custom_Authentication.html
ENG_Building.html
DG_WorkflowReRun.html
DG_SshActionExtension.html
DG_SqoopActionExtension.html
DG_SparkActionExtension.html
DG_SLAMonitoring.html


docs/src/site/twiki/DG_SLAMonitoring.twiki
Line 361 (original), 386 (patched)


html - header level (indentation at the summary)



docs/src/site/twiki/DG_SLAMonitoring.twiki
Line 399 (original), 428 (patched)


html - header level (indentation at the summary)



docs/src/site/twiki/DG_SLAMonitoring.twiki
Line 405 (original), 434 (patched)


html - header level (indentation at the summary)



docs/src/site/twiki/DG_SLAMonitoring.twiki
Line 408 (original), 437 (patched)


html - header level (indentation at the summary)



docs/src/site/twiki/DG_WorkflowReRun.twiki
Lines 10-13 (original), 10-13 (patched)


html


- Kinga Marton


On Sept. 5, 2018, 10 a.m., Andras Salamon wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68628/
> ---
> 
> (Updated Sept. 5, 2018, 10 a.m.)
> 
> 
> Review request for oozie and András Piros.
> 
> 
> Repository: oozie-git
> 
> 
> Description
> ---
> 
> OOZIE-2734 - Switch docs from twiki to markdown
> 
> 
> Diffs
> -
> 
>   docs/pom.xml ae2e94c9c 
>   docs/src/site/markdown/AG_OozieLogging.md PRE-CREATION 
>   docs/src/site/markdown/ENG_Building.md PRE-CREATION 
>   docs/src/site/markdown/index.md PRE-CREATION 
>   docs/src/site/twiki/AG_ActionConfiguration.twiki 8c032a754 
>   docs/src/site/twiki/AG_HadoopConfiguration.twiki 528bf4a41 
>   docs/src/site/twiki/AG_Install.twiki b8031c82e 
>   docs/src/site/twiki/AG_Monitoring.twiki 425f55435 
>   docs/src/site/twiki/AG_OozieLogging.twiki ecdcfd33e 
>   docs/src/site/twiki/AG_OozieUpgrade.twiki 302406482 
>   docs/src/site/twiki/BundleFunctionalSpec.twiki 9749df597 
>   docs/src/site/twiki/CoordinatorFunctionalSpec.twiki cd416d4cd 
>   docs/src/site/twiki/DG_ActionAuthentication.twiki bbf2d57d3 
>   docs/src/site/twiki/DG_CommandLineTool.twiki 8d33c508e 
>   docs/src/site/twiki/DG_CoordinatorRerun.twiki fbb1376d5 
>   docs/src/site/twiki/DG_CustomActionExecutor.twiki 783148495 
>   docs/src/site/twiki/DG_DistCpActionExtension.twiki 8bab3da44 
>   docs/src/site/twiki/DG_EmailActionExtension.twiki 4de290ccf 
>   docs/src/site/twiki/DG_Examples.twiki 5323a1773 
>   docs/src/site/twiki/DG_FluentJobAPI.twiki c8b764bb0 
>   docs/src/site/twiki/DG_HCatalogIntegration.twiki d3107b453 
>   docs/src/site/twiki/DG_Hive2ActionExtension.twiki efbe56dab 
>   docs/src/site/twiki/DG_HiveActionExtension.twiki aaa74fa7b 
>   docs/src/site/twiki/DG_JMSNotifications.twiki a4b0f0d1a 
>   docs/src/site/twiki/DG_Overview.twiki 3ec94a2f1 
>   docs/src/site/twiki/DG_QuickStart.twiki d6a0069ac 
>   docs/src/site/twiki/DG_SLAMonitoring.twiki c91c227dc 
>   docs/src/site/twiki/DG_ShellActionExtension.twiki 5894c280a 
>   docs/src/site/twiki/DG_SparkActionExtension.twiki ce80e45fc 
>   docs/src/site/twiki/DG_SqoopActionExtension.twiki 0317d0cec 
>   docs/src/site/twiki/DG_SshActionExtension.twiki 5a51d49c5 
>   docs/src/site/twiki/DG_WorkflowReRun.twiki 88d982b75 
>   docs/src/site/twiki/ENG_Building.twiki b86102683 
>   docs/src/site/twiki/ENG_Custom_Authentication.twiki 3b8202d0d 
>   docs/src/site/twiki/ENG_MiniOozie.twiki 0b1628986 
>   docs/src/site/twiki/WebServicesAPI.twiki f9008a60b 
>   docs/src/site/twiki/WorkflowFunctionalSpec.twiki 46a454c3a 
>   docs/src/site/twiki/index.twiki 3003fa992 
>   pom.xml 423d19d97 
> 
> 
> Diff: https://reviews.apache.org/r/68628/diff/1/
> 
> 
> Testing
> ---
> 
> HTML generated
> 
> 
> Thanks,
> 
> Andras Salamon
> 
>



Re: Review Request 68517: OOZIE 3210 - revision information is empty

2018-08-27 Thread Kinga Marton via Review Board

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


Ship it!




Ship It!

- Kinga Marton


On Aug. 27, 2018, 2:45 p.m., Andras Salamon wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68517/
> ---
> 
> (Updated Aug. 27, 2018, 2:45 p.m.)
> 
> 
> Review request for oozie, András Piros, Attila Sasvari, and Kinga Marton.
> 
> 
> Repository: oozie-git
> 
> 
> Description
> ---
> 
> OOZIE 3210 - revision information is empty
> 
> 
> Diffs
> -
> 
>   bin/create-release-artifact 86d4d5df 
> 
> 
> Diff: https://reviews.apache.org/r/68517/diff/2/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Andras Salamon
> 
>



Re: Review Request 68517: OOZIE 3210 - revision information is empty

2018-08-27 Thread Kinga Marton via Review Board

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




bin/create-release-artifact
Lines 60-69 (patched)


For better readability can you please extract this into a method?


- Kinga Marton


On Aug. 27, 2018, 9:26 a.m., Andras Salamon wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68517/
> ---
> 
> (Updated Aug. 27, 2018, 9:26 a.m.)
> 
> 
> Review request for oozie, András Piros, Attila Sasvari, and Kinga Marton.
> 
> 
> Repository: oozie-git
> 
> 
> Description
> ---
> 
> OOZIE 3210 - revision information is empty
> 
> 
> Diffs
> -
> 
>   bin/create-release-artifact 86d4d5df 
> 
> 
> Diff: https://reviews.apache.org/r/68517/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Andras Salamon
> 
>



Re: Review Request 68467: OOZIE-2684 Bad database schema error for WF_ACTIONS table

2018-08-27 Thread Kinga Marton via Review Board

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

(Updated Aug. 27, 2018, 11:35 a.m.)


Review request for oozie, András Piros and Andras Salamon.


Changes
---

addressed FindBugs issues


Bugs: OOZIE-2684
https://issues.apache.org/jira/browse/OOZIE-2684


Repository: oozie-git


Description
---

In SchemaCheckerService, Oozie compares expected and found indexed columns and 
generates the below error message which could be confusing to the users.

`2016-09-16 12:39:26,564  INFO SchemaCheckXCommand:520 - 
SERVER[c6402.ambari.apache.org] USER[-] GROUP[-] TOKEN[-] APP[-] JOB[-] 
ACTION[-] About to check database schema`
`2016-09-16 12:39:26,703 ERROR SchemaCheckXCommand:517 - 
SERVER[c6402.ambari.apache.org] USER[-] GROUP[-] TOKEN[-] APP[-] JOB[-] 
ACTION[-] Found [1] extra indexes for columns in table [WF_ACTIONS]: [wf_id]`
`2016-09-16 12:39:26,723 ERROR SchemaCheckXCommand:517 - 
SERVER[c6402.ambari.apache.org] USER[-] GROUP[-] TOKEN[-] APP[-] JOB[-] 
ACTION[-] Database schema is BAD! Check previous error log messages for details`


Diffs (updated)
-

  core/src/main/java/org/apache/oozie/command/SchemaCheckXCommand.java 521408ee 
  core/src/main/java/org/apache/oozie/util/db/CompositeIndex.java PRE-CREATION 
  core/src/test/java/org/apache/oozie/util/db/TestCompositeIndex.java 
PRE-CREATION 
  tools/src/main/java/org/apache/oozie/tools/OozieDBCLI.java eeaaa60c 
  tools/src/test/java/org/apache/oozie/tools/TestOozieDBCLI.java cf3427f3 


Diff: https://reviews.apache.org/r/68467/diff/3/

Changes: https://reviews.apache.org/r/68467/diff/2-3/


Testing
---

Tested manually with MySql and PostgreSql.

I will open another Jira for making this part of code more testable, since now 
is quite difficult to write some tests for it.


Thanks,

Kinga Marton



Re: Review Request 68467: OOZIE-2684 Bad database schema error for WF_ACTIONS table

2018-08-24 Thread Kinga Marton via Review Board

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

(Updated Aug. 24, 2018, 1:36 p.m.)


Review request for oozie, András Piros and Andras Salamon.


Changes
---

review fix


Bugs: OOZIE-2684
https://issues.apache.org/jira/browse/OOZIE-2684


Repository: oozie-git


Description
---

In SchemaCheckerService, Oozie compares expected and found indexed columns and 
generates the below error message which could be confusing to the users.

`2016-09-16 12:39:26,564  INFO SchemaCheckXCommand:520 - 
SERVER[c6402.ambari.apache.org] USER[-] GROUP[-] TOKEN[-] APP[-] JOB[-] 
ACTION[-] About to check database schema`
`2016-09-16 12:39:26,703 ERROR SchemaCheckXCommand:517 - 
SERVER[c6402.ambari.apache.org] USER[-] GROUP[-] TOKEN[-] APP[-] JOB[-] 
ACTION[-] Found [1] extra indexes for columns in table [WF_ACTIONS]: [wf_id]`
`2016-09-16 12:39:26,723 ERROR SchemaCheckXCommand:517 - 
SERVER[c6402.ambari.apache.org] USER[-] GROUP[-] TOKEN[-] APP[-] JOB[-] 
ACTION[-] Database schema is BAD! Check previous error log messages for details`


Diffs (updated)
-

  core/src/main/java/org/apache/oozie/command/SchemaCheckXCommand.java 521408ee 
  core/src/main/java/org/apache/oozie/util/db/CompositeIndex.java PRE-CREATION 
  core/src/test/java/org/apache/oozie/util/db/TestCompositeIndex.java 
PRE-CREATION 
  tools/src/main/java/org/apache/oozie/tools/OozieDBCLI.java eeaaa60c 
  tools/src/test/java/org/apache/oozie/tools/TestOozieDBCLI.java cf3427f3 


Diff: https://reviews.apache.org/r/68467/diff/2/

Changes: https://reviews.apache.org/r/68467/diff/1-2/


Testing
---

Tested manually with MySql and PostgreSql.

I will open another Jira for making this part of code more testable, since now 
is quite difficult to write some tests for it.


Thanks,

Kinga Marton



Review Request 68467: OOZIE-2684 Bad database schema error for WF_ACTIONS table

2018-08-22 Thread Kinga Marton via Review Board

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

Review request for oozie, András Piros and Andras Salamon.


Bugs: OOZIE-2684
https://issues.apache.org/jira/browse/OOZIE-2684


Repository: oozie-git


Description
---

In SchemaCheckerService, Oozie compares expected and found indexed columns and 
generates the below error message which could be confusing to the users.

`2016-09-16 12:39:26,564  INFO SchemaCheckXCommand:520 - 
SERVER[c6402.ambari.apache.org] USER[-] GROUP[-] TOKEN[-] APP[-] JOB[-] 
ACTION[-] About to check database schema`
`2016-09-16 12:39:26,703 ERROR SchemaCheckXCommand:517 - 
SERVER[c6402.ambari.apache.org] USER[-] GROUP[-] TOKEN[-] APP[-] JOB[-] 
ACTION[-] Found [1] extra indexes for columns in table [WF_ACTIONS]: [wf_id]`
`2016-09-16 12:39:26,723 ERROR SchemaCheckXCommand:517 - 
SERVER[c6402.ambari.apache.org] USER[-] GROUP[-] TOKEN[-] APP[-] JOB[-] 
ACTION[-] Database schema is BAD! Check previous error log messages for details`


Diffs
-

  core/src/main/java/org/apache/oozie/command/SchemaCheckXCommand.java 521408ee 
  core/src/main/java/org/apache/oozie/util/db/CompositeIndex.java PRE-CREATION 
  core/src/test/java/org/apache/oozie/util/db/TestCompositeIndex.java 
PRE-CREATION 
  tools/src/main/java/org/apache/oozie/tools/OozieDBCLI.java eeaaa60c 


Diff: https://reviews.apache.org/r/68467/diff/1/


Testing
---

Tested manually with MySql and PostgreSql.

I will open another Jira for making this part of code more testable, since now 
is quite difficult to write some tests for it.


Thanks,

Kinga Marton



Re: Review Request 68180: OOZIE-3315 Datelist Java Main example fails

2018-08-03 Thread Kinga Marton via Review Board


> On Aug. 3, 2018, 8 a.m., Kinga Marton wrote:
> > examples/src/main/java/org/apache/oozie/example/DateList.java
> > Line 21 (original), 21 (patched)
> > 
> >
> > Please revove * import

*remove :)


- Kinga


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


On Aug. 3, 2018, 6:38 a.m., Daniel Becker wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68180/
> ---
> 
> (Updated Aug. 3, 2018, 6:38 a.m.)
> 
> 
> Review request for oozie and András Piros.
> 
> 
> Repository: oozie-git
> 
> 
> Description
> ---
> 
> The oozie/examples/src/main/java/org/apache/oozie/example/DateList.java 
> produces a wrong action output:
> 
> ,2009-02-01T01:15Z,2009-02-01T01:30Z,2009-02-01T01:45Z,2009-02-01T02:00Z
> instead of
> 
> 2009-02-01T01:00Z,2009-02-01T01:15Z,2009-02-01T01:30Z,2009-02-01T01:45Z
> the first element is missing in the list (but not the separator). This is 
> caused by an off-by-one error introduced in OOZIE-2942.
> 
> 
> Diffs
> -
> 
>   examples/pom.xml 680e3fb1 
>   examples/src/main/java/org/apache/oozie/example/DateList.java 731fe413 
>   examples/src/test/java/org/apache/oozie/example/TestDateList.java 
> PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/68180/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Daniel Becker
> 
>



Re: Review Request 68180: OOZIE-3315 Datelist Java Main example fails

2018-08-03 Thread Kinga Marton via Review Board

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




examples/src/main/java/org/apache/oozie/example/DateList.java
Line 21 (original), 21 (patched)


Please revove * import


- Kinga Marton


On Aug. 3, 2018, 6:38 a.m., Daniel Becker wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68180/
> ---
> 
> (Updated Aug. 3, 2018, 6:38 a.m.)
> 
> 
> Review request for oozie and András Piros.
> 
> 
> Repository: oozie-git
> 
> 
> Description
> ---
> 
> The oozie/examples/src/main/java/org/apache/oozie/example/DateList.java 
> produces a wrong action output:
> 
> ,2009-02-01T01:15Z,2009-02-01T01:30Z,2009-02-01T01:45Z,2009-02-01T02:00Z
> instead of
> 
> 2009-02-01T01:00Z,2009-02-01T01:15Z,2009-02-01T01:30Z,2009-02-01T01:45Z
> the first element is missing in the list (but not the separator). This is 
> caused by an off-by-one error introduced in OOZIE-2942.
> 
> 
> Diffs
> -
> 
>   examples/pom.xml 680e3fb1 
>   examples/src/main/java/org/apache/oozie/example/DateList.java 731fe413 
>   examples/src/test/java/org/apache/oozie/example/TestDateList.java 
> PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/68180/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Daniel Becker
> 
>



Re: Review Request 68102: OOZIE-3193 Applications are not killed when submitted via subworkflow

2018-08-01 Thread Kinga Marton via Review Board

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

(Updated Aug. 1, 2018, 1:44 p.m.)


Review request for oozie, András Piros and Peter Cseh.


Repository: oozie-git


Description
---

Tags were not generated properly for external child jobs launched from subWFs 
and they were not killed.


Diffs (updated)
-

  core/src/main/java/org/apache/oozie/action/ActionExecutor.java 71bd36ae6 
  core/src/main/java/org/apache/oozie/action/hadoop/JavaActionExecutor.java 
a1a9671ab 
  core/src/test/java/org/apache/oozie/action/hadoop/LauncherMainTester.java 
fdca5706e 
  
core/src/test/java/org/apache/oozie/action/hadoop/SleepMapperReducerForTest.java
 PRE-CREATION 
  
core/src/test/java/org/apache/oozie/action/oozie/TestSubWorkflowActionExecutor.java
 e074d482f 


Diff: https://reviews.apache.org/r/68102/diff/3/

Changes: https://reviews.apache.org/r/68102/diff/2-3/


Testing
---

Tested manually.
Added Junit test.


Thanks,

Kinga Marton



Re: Review Request 68102: OOZIE-3193 Applications are not killed when submitted via subworkflow

2018-08-01 Thread Kinga Marton via Review Board


> On July 30, 2018, 10:46 a.m., András Piros wrote:
> > core/src/test/java/org/apache/oozie/action/hadoop/SleepMapperReducerForTest.java
> > Lines 60-61 (patched)
> > 
> >
> > Isn't emitting another message also just after sleep a good idea?

I have added a wake up message as well :)


> On July 30, 2018, 10:46 a.m., András Piros wrote:
> > core/src/test/java/org/apache/oozie/action/oozie/TestSubWorkflowActionExecutor.java
> > Lines 555-574 (patched)
> > 
> >
> > Please extract to `private static class ApplicationIdExistsPredicate 
> > implements Predicate`.

I have extracted only the part performing the check.


> On July 30, 2018, 10:46 a.m., András Piros wrote:
> > core/src/test/java/org/apache/oozie/action/oozie/TestSubWorkflowActionExecutor.java
> > Lines 708-712 (patched)
> > 
> >
> > Please use latest schema version `1.0`.
> 
> Peter Cseh wrote:
> I don't think we should give up test coverage for older schemas. We 
> should keep around the old ones in test and make sure to add enough covergave 
> for new schemas when we add them.

I can change here to the new schema, since there are some similar workflows 
using the old schema.


- Kinga


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


On Aug. 1, 2018, 9:59 a.m., Kinga Marton wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68102/
> ---
> 
> (Updated Aug. 1, 2018, 9:59 a.m.)
> 
> 
> Review request for oozie, András Piros and Peter Cseh.
> 
> 
> Repository: oozie-git
> 
> 
> Description
> ---
> 
> Tags were not generated properly for external child jobs launched from subWFs 
> and they were not killed.
> 
> 
> Diffs
> -
> 
>   core/src/main/java/org/apache/oozie/action/ActionExecutor.java 71bd36ae6 
>   core/src/main/java/org/apache/oozie/action/hadoop/JavaActionExecutor.java 
> a1a9671ab 
>   core/src/test/java/org/apache/oozie/action/hadoop/LauncherMainTester.java 
> fdca5706e 
>   
> core/src/test/java/org/apache/oozie/action/hadoop/SleepMapperReducerForTest.java
>  PRE-CREATION 
>   
> core/src/test/java/org/apache/oozie/action/oozie/TestSubWorkflowActionExecutor.java
>  e074d482f 
> 
> 
> Diff: https://reviews.apache.org/r/68102/diff/2/
> 
> 
> Testing
> ---
> 
> Tested manually.
> Added Junit test.
> 
> 
> Thanks,
> 
> Kinga Marton
> 
>



Re: Review Request 68102: OOZIE-3193 Applications are not killed when submitted via subworkflow

2018-08-01 Thread Kinga Marton via Review Board

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

(Updated Aug. 1, 2018, 9:59 a.m.)


Review request for oozie, András Piros and Peter Cseh.


Repository: oozie-git


Description
---

Tags were not generated properly for external child jobs launched from subWFs 
and they were not killed.


Diffs (updated)
-

  core/src/main/java/org/apache/oozie/action/ActionExecutor.java 71bd36ae6 
  core/src/main/java/org/apache/oozie/action/hadoop/JavaActionExecutor.java 
a1a9671ab 
  core/src/test/java/org/apache/oozie/action/hadoop/LauncherMainTester.java 
fdca5706e 
  
core/src/test/java/org/apache/oozie/action/hadoop/SleepMapperReducerForTest.java
 PRE-CREATION 
  
core/src/test/java/org/apache/oozie/action/oozie/TestSubWorkflowActionExecutor.java
 e074d482f 


Diff: https://reviews.apache.org/r/68102/diff/2/

Changes: https://reviews.apache.org/r/68102/diff/1-2/


Testing
---

Tested manually.
Added Junit test.


Thanks,

Kinga Marton



Review Request 68102: OOZIE-3193 Applications are not killed when submitted via subworkflow

2018-07-30 Thread Kinga Marton via Review Board

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

Review request for oozie, András Piros and Peter Cseh.


Repository: oozie-git


Description
---

Tags were not generated properly for external child jobs launched from subWFs 
and they were not killed.


Diffs
-

  core/src/main/java/org/apache/oozie/action/ActionExecutor.java 71bd36ae6 
  core/src/main/java/org/apache/oozie/action/hadoop/JavaActionExecutor.java 
a1a9671ab 
  core/src/test/java/org/apache/oozie/action/hadoop/LauncherMainTester.java 
fdca5706e 
  
core/src/test/java/org/apache/oozie/action/hadoop/SleepMapperReducerForTest.java
 PRE-CREATION 
  
core/src/test/java/org/apache/oozie/action/oozie/TestSubWorkflowActionExecutor.java
 e074d482f 


Diff: https://reviews.apache.org/r/68102/diff/1/


Testing
---

Tested manually.
Added Junit test.


Thanks,

Kinga Marton



Re: Review Request 67834: OOZIE-2829 Improve sharelib upload to accept multiple source folders

2018-07-25 Thread Kinga Marton via Review Board

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

(Updated July 25, 2018, 8:48 a.m.)


Review request for oozie, András Piros, Peter Cseh, and Peter Bacsko.


Changes
---

multiple options for multiple sharelibs added


Repository: oozie-git


Description
---

Right now sharelib can be created via sharelib create -fs FS_URI -locallib 
SHARED_LIBRARY where the SHARED_LIBRARY can be a tarbal or a folder.
It would be nice to have the possibility to define additional folders to be 
uploaded into the sharelib, so the users don't have to copy or link the files 
together on their machine.
The syntax could be something like -additional-lib 
sharelibName=/path/to/source/;/path/to/some/file,sharelibName2=/path/to/some/folder


Diffs (updated)
-

  docs/src/site/twiki/AG_Install.twiki 46363a3b3 
  tools/src/main/java/org/apache/oozie/tools/OozieSharelibCLI.java 75e932c02 
  
tools/src/test/java/org/apache/oozie/tools/IntegrationTestOozieSharelibCLI.java 
PRE-CREATION 
  tools/src/test/java/org/apache/oozie/tools/OozieSharelibFileOperations.java 
d344300ed 
  tools/src/test/java/org/apache/oozie/tools/TestConcurrentCopyFromLocal.java 
d77eba69d 
  tools/src/test/java/org/apache/oozie/tools/TestCopyTaskCallable.java 
bce0433c6 
  tools/src/test/java/org/apache/oozie/tools/TestOozieSharelibCLI.java 
5929e5ca0 
  
tools/src/test/java/org/apache/oozie/tools/TestOozieSharelibCLIExtraArgsParser.java
 PRE-CREATION 


Diff: https://reviews.apache.org/r/67834/diff/7/

Changes: https://reviews.apache.org/r/67834/diff/6-7/


Testing
---

Tested manually + added integration test


Thanks,

Kinga Marton



Re: Review Request 67834: OOZIE-2829 Improve sharelib upload to accept multiple source folders

2018-07-22 Thread Kinga Marton via Review Board

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

(Updated July 22, 2018, 4 p.m.)


Review request for oozie, András Piros, Peter Cseh, and Peter Bacsko.


Changes
---

test failure fix


Repository: oozie-git


Description
---

Right now sharelib can be created via sharelib create -fs FS_URI -locallib 
SHARED_LIBRARY where the SHARED_LIBRARY can be a tarbal or a folder.
It would be nice to have the possibility to define additional folders to be 
uploaded into the sharelib, so the users don't have to copy or link the files 
together on their machine.
The syntax could be something like -additional-lib 
sharelibName=/path/to/source/;/path/to/some/file,sharelibName2=/path/to/some/folder


Diffs (updated)
-

  docs/src/site/twiki/AG_Install.twiki 46363a3b3 
  tools/src/main/java/org/apache/oozie/tools/OozieSharelibCLI.java 75e932c02 
  
tools/src/test/java/org/apache/oozie/tools/IntegrationTestOozieSharelibCLI.java 
PRE-CREATION 
  tools/src/test/java/org/apache/oozie/tools/OozieSharelibFileOperations.java 
d344300ed 
  tools/src/test/java/org/apache/oozie/tools/TestConcurrentCopyFromLocal.java 
d77eba69d 
  tools/src/test/java/org/apache/oozie/tools/TestCopyTaskCallable.java 
bce0433c6 
  tools/src/test/java/org/apache/oozie/tools/TestOozieSharelibCLI.java 
5929e5ca0 
  
tools/src/test/java/org/apache/oozie/tools/TestOozieSharelibCLIExtraArgsParser.java
 PRE-CREATION 


Diff: https://reviews.apache.org/r/67834/diff/6/

Changes: https://reviews.apache.org/r/67834/diff/5-6/


Testing
---

Tested manually + added integration test


Thanks,

Kinga Marton



Re: Review Request 67834: OOZIE-2829 Improve sharelib upload to accept multiple source folders

2018-07-22 Thread Kinga Marton via Review Board

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

(Updated July 22, 2018, 1:22 p.m.)


Review request for oozie, András Piros, Peter Cseh, and Peter Bacsko.


Changes
---

- fixed review comments
- added some additional tests
- changed the sharelibs separator to '+'. However I am not completely satisfied 
with this solution, but I don't have any better ideas.


Repository: oozie-git


Description
---

Right now sharelib can be created via sharelib create -fs FS_URI -locallib 
SHARED_LIBRARY where the SHARED_LIBRARY can be a tarbal or a folder.
It would be nice to have the possibility to define additional folders to be 
uploaded into the sharelib, so the users don't have to copy or link the files 
together on their machine.
The syntax could be something like -additional-lib 
sharelibName=/path/to/source/;/path/to/some/file,sharelibName2=/path/to/some/folder


Diffs (updated)
-

  docs/src/site/twiki/AG_Install.twiki 46363a3b3 
  tools/src/main/java/org/apache/oozie/tools/OozieSharelibCLI.java 75e932c02 
  
tools/src/test/java/org/apache/oozie/tools/IntegrationTestOozieSharelibCLI.java 
PRE-CREATION 
  tools/src/test/java/org/apache/oozie/tools/OozieSharelibFileOperations.java 
d344300ed 
  tools/src/test/java/org/apache/oozie/tools/TestConcurrentCopyFromLocal.java 
d77eba69d 
  tools/src/test/java/org/apache/oozie/tools/TestCopyTaskCallable.java 
bce0433c6 
  tools/src/test/java/org/apache/oozie/tools/TestOozieSharelibCLI.java 
5929e5ca0 
  
tools/src/test/java/org/apache/oozie/tools/TestOozieSharelibCLIExtraArgsParser.java
 PRE-CREATION 


Diff: https://reviews.apache.org/r/67834/diff/5/

Changes: https://reviews.apache.org/r/67834/diff/4-5/


Testing
---

Tested manually + added integration test


Thanks,

Kinga Marton



Review Request 68008: OOZIE-2942 [examples] Fix Findbugs warnings

2018-07-22 Thread Kinga Marton via Review Board

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

Review request for oozie, András Piros and Peter Cseh.


Repository: oozie-git


Description
---

Currently Findbugs complains about the following warnings in the oozie-examples 
module:
```
[INFO] org.apache.oozie.example.DateList.main(String[]) may fail to close 
stream on exception [org.apache.oozie.example.DateList] At DateList.java:[line 
69]
[INFO] Nullcheck of date at line 55 of value previously dereferenced in 
org.apache.oozie.example.DateList.main(String[]) 
[org.apache.oozie.example.DateList, org.apache.oozie.example.DateList] At 
DateList.java:[line 55]Redundant null check at DateList.java:[line 62]
[INFO] Private method org.apache.oozie.example.DateList.formatDateUTC(Calendar) 
is never called [org.apache.oozie.example.DateList] At DateList.java:[line 97]
[INFO] org.apache.oozie.example.LocalOozieExample.execute(String[]) may fail to 
clean up java.io.InputStream [org.apache.oozie.example.LocalOozieExample, 
org.apache.oozie.example.LocalOozieExample, 
org.apache.oozie.example.LocalOozieExample, 
org.apache.oozie.example.LocalOozieExample, 
org.apache.oozie.example.LocalOozieExample, 
org.apache.oozie.example.LocalOozieExample, 
org.apache.oozie.example.LocalOozieExample, 
org.apache.oozie.example.LocalOozieExample, 
org.apache.oozie.example.LocalOozieExample] Obligation to clean up resource 
created at LocalOozieExample.java:[line 72] is not dischargedPath continues at 
LocalOozieExample.java:[line 76]Path continues at LocalOozieExample.java:[line 
77]Path continues at LocalOozieExample.java:[line 78]Path continues at 
LocalOozieExample.java:[line 81]Path continues at LocalOozieExample.java:[line 
88]Path continues at LocalOozieExample.java:[line 89]Path continues at 
LocalOozieExample.java:[line 91]Path continues at LocalOozieExample.java:[line 
10
 0]
[INFO] org.apache.oozie.example.LocalOozieExample.execute(String[]) may fail to 
close stream [org.apache.oozie.example.LocalOozieExample] At 
LocalOozieExample.java:[line 72]
[INFO] org.apache.oozie.example.Repeatable.getBaseline() may expose internal 
representation by returning Repeatable.baseline 
[org.apache.oozie.example.Repeatable] At Repeatable.java:[line 168]
[INFO] org.apache.oozie.example.Repeatable.setBaseline(Date) may expose 
internal representation by storing an externally mutable object into 
Repeatable.baseline [org.apache.oozie.example.Repeatable] At 
Repeatable.java:[line 172]
```

They should be fixed to get the code more reliable.


Diffs
-

  examples/src/main/java/org/apache/oozie/example/DateList.java 7e574cbe 
  examples/src/main/java/org/apache/oozie/example/LocalOozieExample.java 
c9f5697c 
  examples/src/main/java/org/apache/oozie/example/Repeatable.java ee863251 


Diff: https://reviews.apache.org/r/68008/diff/1/


Testing
---


Thanks,

Kinga Marton



Re: Review Request 67834: OOZIE-2829 Improve sharelib upload to accept multiple source folders

2018-07-22 Thread Kinga Marton via Review Board


> On July 6, 2018, 1:40 p.m., András Piros wrote:
> > tools/src/main/java/org/apache/oozie/tools/OozieSharelibCLI.java
> > Lines 88 (patched)
> > 
> >
> > Problem is we can have ```hdfs://my/jar.jar#myjar.jar``` inside 
> > sharelib paths.
> > 
> > Best is to go with e.g. `;` that cannot be present as HDFS or local 
> > file.
> 
> Kinga Marton wrote:
> Using semicolon(;) is not a good ideea because the shell handles it as a 
> command separator. What about using & ?
> 
> András Piros wrote:
> I'm OK using colon `,` as the separator between sharelib name-path pairs, 
> and pipe `|` between sharelib paths like this:
> ```
> additional-lib 
> sharelibName1=/path/to/source/|/path/to/some/file,sharelibName2=/path/to/some/folder
> ```
> 
> Kinga Marton wrote:
> The pipe will not work, since it has the same problem as the semicolon. 
> It is interpreted by the Shell. It can work only if is surrounded by double 
> quotes. The problem is that every character that seems to be logic as a 
> delimiter, is interpreted by the Shell. 
> What about using '+' as sharelibs delimiter?
> Using '+' would result in the following command:
> ```
> -extralibs 
> sharelibName=/path/to/source/,/path/to/some/file+sharelibName2=/path/to/some/folder,hdfs://my/jar.jar#myjar.jar
> ```
> 
> Or another solution would be to use double quotes if we want to specify 
> more than one extra sharelib. This way we can even use the semicolon (;)

I have changes the ":" to "+", since I dont't have any better idea. However I 
am not 100% satisfied with this solution.


- Kinga


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


On July 6, 2018, 8:36 a.m., Kinga Marton wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67834/
> ---
> 
> (Updated July 6, 2018, 8:36 a.m.)
> 
> 
> Review request for oozie, András Piros, Peter Cseh, and Peter Bacsko.
> 
> 
> Repository: oozie-git
> 
> 
> Description
> ---
> 
> Right now sharelib can be created via sharelib create -fs FS_URI -locallib 
> SHARED_LIBRARY where the SHARED_LIBRARY can be a tarbal or a folder.
> It would be nice to have the possibility to define additional folders to be 
> uploaded into the sharelib, so the users don't have to copy or link the files 
> together on their machine.
> The syntax could be something like -additional-lib 
> sharelibName=/path/to/source/;/path/to/some/file,sharelibName2=/path/to/some/folder
> 
> 
> Diffs
> -
> 
>   docs/src/site/twiki/AG_Install.twiki 46363a3b3 
>   tools/src/main/java/org/apache/oozie/tools/OozieSharelibCLI.java 75e932c02 
>   tools/src/test/java/org/apache/oozie/tools/OozieSharelibFileOperations.java 
> d344300ed 
>   tools/src/test/java/org/apache/oozie/tools/TestConcurrentCopyFromLocal.java 
> d77eba69d 
>   tools/src/test/java/org/apache/oozie/tools/TestCopyTaskCallable.java 
> bce0433c6 
>   tools/src/test/java/org/apache/oozie/tools/TestOozieSharelibCLI.java 
> 5929e5ca0 
>   
> tools/src/test/java/org/apache/oozie/tools/diag/IntegrationTestOozieSharelibCLI.java
>  PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/67834/diff/4/
> 
> 
> Testing
> ---
> 
> Tested manually + added integration test
> 
> 
> Thanks,
> 
> Kinga Marton
> 
>



Re: Review Request 67834: OOZIE-2829 Improve sharelib upload to accept multiple source folders

2018-07-20 Thread Kinga Marton via Review Board


> On July 6, 2018, 1:40 p.m., András Piros wrote:
> > tools/src/main/java/org/apache/oozie/tools/OozieSharelibCLI.java
> > Lines 88 (patched)
> > 
> >
> > Problem is we can have ```hdfs://my/jar.jar#myjar.jar``` inside 
> > sharelib paths.
> > 
> > Best is to go with e.g. `;` that cannot be present as HDFS or local 
> > file.
> 
> Kinga Marton wrote:
> Using semicolon(;) is not a good ideea because the shell handles it as a 
> command separator. What about using & ?
> 
> András Piros wrote:
> I'm OK using colon `,` as the separator between sharelib name-path pairs, 
> and pipe `|` between sharelib paths like this:
> ```
> additional-lib 
> sharelibName1=/path/to/source/|/path/to/some/file,sharelibName2=/path/to/some/folder
> ```

The pipe will not work, since it has the same problem as the semicolon. It is 
interpreted by the Shell. It can work only if is surrounded by double quotes. 
The problem is that every character that seems to be logic as a delimiter, is 
interpreted by the Shell. 
What about using '+' as sharelibs delimiter?
Using '+' would result in the following command:
```
-extralibs 
sharelibName=/path/to/source/,/path/to/some/file+sharelibName2=/path/to/some/folder,hdfs://my/jar.jar#myjar.jar
```

Or another solution would be to use double quotes if we want to specify more 
than one extra sharelib. This way we can even use the semicolon (;)


- Kinga


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


On July 6, 2018, 8:36 a.m., Kinga Marton wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67834/
> ---
> 
> (Updated July 6, 2018, 8:36 a.m.)
> 
> 
> Review request for oozie, András Piros, Peter Cseh, and Peter Bacsko.
> 
> 
> Repository: oozie-git
> 
> 
> Description
> ---
> 
> Right now sharelib can be created via sharelib create -fs FS_URI -locallib 
> SHARED_LIBRARY where the SHARED_LIBRARY can be a tarbal or a folder.
> It would be nice to have the possibility to define additional folders to be 
> uploaded into the sharelib, so the users don't have to copy or link the files 
> together on their machine.
> The syntax could be something like -additional-lib 
> sharelibName=/path/to/source/;/path/to/some/file,sharelibName2=/path/to/some/folder
> 
> 
> Diffs
> -
> 
>   docs/src/site/twiki/AG_Install.twiki 46363a3b3 
>   tools/src/main/java/org/apache/oozie/tools/OozieSharelibCLI.java 75e932c02 
>   tools/src/test/java/org/apache/oozie/tools/OozieSharelibFileOperations.java 
> d344300ed 
>   tools/src/test/java/org/apache/oozie/tools/TestConcurrentCopyFromLocal.java 
> d77eba69d 
>   tools/src/test/java/org/apache/oozie/tools/TestCopyTaskCallable.java 
> bce0433c6 
>   tools/src/test/java/org/apache/oozie/tools/TestOozieSharelibCLI.java 
> 5929e5ca0 
>   
> tools/src/test/java/org/apache/oozie/tools/diag/IntegrationTestOozieSharelibCLI.java
>  PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/67834/diff/4/
> 
> 
> Testing
> ---
> 
> Tested manually + added integration test
> 
> 
> Thanks,
> 
> Kinga Marton
> 
>



Re: Review Request 67834: OOZIE-2829 Improve sharelib upload to accept multiple source folders

2018-07-19 Thread Kinga Marton via Review Board


> On July 6, 2018, 1:40 p.m., András Piros wrote:
> > tools/src/main/java/org/apache/oozie/tools/OozieSharelibCLI.java
> > Lines 88 (patched)
> > 
> >
> > Problem is we can have ```hdfs://my/jar.jar#myjar.jar``` inside 
> > sharelib paths.
> > 
> > Best is to go with e.g. `;` that cannot be present as HDFS or local 
> > file.

Using semicolon(;) is not a good ideea because the shell handles it as a 
command separator. What about using & ?


- Kinga


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


On July 6, 2018, 8:36 a.m., Kinga Marton wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67834/
> ---
> 
> (Updated July 6, 2018, 8:36 a.m.)
> 
> 
> Review request for oozie, András Piros, Peter Cseh, and Peter Bacsko.
> 
> 
> Repository: oozie-git
> 
> 
> Description
> ---
> 
> Right now sharelib can be created via sharelib create -fs FS_URI -locallib 
> SHARED_LIBRARY where the SHARED_LIBRARY can be a tarbal or a folder.
> It would be nice to have the possibility to define additional folders to be 
> uploaded into the sharelib, so the users don't have to copy or link the files 
> together on their machine.
> The syntax could be something like -additional-lib 
> sharelibName=/path/to/source/;/path/to/some/file,sharelibName2=/path/to/some/folder
> 
> 
> Diffs
> -
> 
>   docs/src/site/twiki/AG_Install.twiki 46363a3b3 
>   tools/src/main/java/org/apache/oozie/tools/OozieSharelibCLI.java 75e932c02 
>   tools/src/test/java/org/apache/oozie/tools/OozieSharelibFileOperations.java 
> d344300ed 
>   tools/src/test/java/org/apache/oozie/tools/TestConcurrentCopyFromLocal.java 
> d77eba69d 
>   tools/src/test/java/org/apache/oozie/tools/TestCopyTaskCallable.java 
> bce0433c6 
>   tools/src/test/java/org/apache/oozie/tools/TestOozieSharelibCLI.java 
> 5929e5ca0 
>   
> tools/src/test/java/org/apache/oozie/tools/diag/IntegrationTestOozieSharelibCLI.java
>  PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/67834/diff/4/
> 
> 
> Testing
> ---
> 
> Tested manually + added integration test
> 
> 
> Thanks,
> 
> Kinga Marton
> 
>



Re: Review Request 67834: OOZIE-2829 Improve sharelib upload to accept multiple source folders

2018-07-08 Thread Kinga Marton via Review Board


> On July 6, 2018, 1:40 p.m., András Piros wrote:
> > tools/src/test/java/org/apache/oozie/tools/OozieSharelibFileOperations.java
> > Lines 42-48 (original), 43-51 (patched)
> > 
> >
> > Could be extracted to a nested `static class`, and tested separately.

I agree with testing it, but this class was created for the Sharelib testing 
related file operations, like this method.


- Kinga


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


On July 6, 2018, 8:36 a.m., Kinga Marton wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67834/
> ---
> 
> (Updated July 6, 2018, 8:36 a.m.)
> 
> 
> Review request for oozie, András Piros, Peter Cseh, and Peter Bacsko.
> 
> 
> Repository: oozie-git
> 
> 
> Description
> ---
> 
> Right now sharelib can be created via sharelib create -fs FS_URI -locallib 
> SHARED_LIBRARY where the SHARED_LIBRARY can be a tarbal or a folder.
> It would be nice to have the possibility to define additional folders to be 
> uploaded into the sharelib, so the users don't have to copy or link the files 
> together on their machine.
> The syntax could be something like -additional-lib 
> sharelibName=/path/to/source/;/path/to/some/file,sharelibName2=/path/to/some/folder
> 
> 
> Diffs
> -
> 
>   docs/src/site/twiki/AG_Install.twiki 46363a3b3 
>   tools/src/main/java/org/apache/oozie/tools/OozieSharelibCLI.java 75e932c02 
>   tools/src/test/java/org/apache/oozie/tools/OozieSharelibFileOperations.java 
> d344300ed 
>   tools/src/test/java/org/apache/oozie/tools/TestConcurrentCopyFromLocal.java 
> d77eba69d 
>   tools/src/test/java/org/apache/oozie/tools/TestCopyTaskCallable.java 
> bce0433c6 
>   tools/src/test/java/org/apache/oozie/tools/TestOozieSharelibCLI.java 
> 5929e5ca0 
>   
> tools/src/test/java/org/apache/oozie/tools/diag/IntegrationTestOozieSharelibCLI.java
>  PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/67834/diff/4/
> 
> 
> Testing
> ---
> 
> Tested manually + added integration test
> 
> 
> Thanks,
> 
> Kinga Marton
> 
>



Re: Review Request 67834: OOZIE-2829 Improve sharelib upload to accept multiple source folders

2018-07-08 Thread Kinga Marton via Review Board


> On July 6, 2018, 1:40 p.m., András Piros wrote:
> > tools/src/main/java/org/apache/oozie/tools/OozieSharelibCLI.java
> > Lines 282-294 (patched)
> > 
> >
> > A log message about how parallel we are running would be nice.

if we are running on multiple threads there is a log message printed out in 
ConcurrentCopyFromLocal.concurrentCopyFromLocal(FileSystem fs, File srcFile, 
Path dstPath). Adding here another log message I think it would be redundant.


- Kinga


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


On July 6, 2018, 8:36 a.m., Kinga Marton wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67834/
> ---
> 
> (Updated July 6, 2018, 8:36 a.m.)
> 
> 
> Review request for oozie, András Piros, Peter Cseh, and Peter Bacsko.
> 
> 
> Repository: oozie-git
> 
> 
> Description
> ---
> 
> Right now sharelib can be created via sharelib create -fs FS_URI -locallib 
> SHARED_LIBRARY where the SHARED_LIBRARY can be a tarbal or a folder.
> It would be nice to have the possibility to define additional folders to be 
> uploaded into the sharelib, so the users don't have to copy or link the files 
> together on their machine.
> The syntax could be something like -additional-lib 
> sharelibName=/path/to/source/;/path/to/some/file,sharelibName2=/path/to/some/folder
> 
> 
> Diffs
> -
> 
>   docs/src/site/twiki/AG_Install.twiki 46363a3b3 
>   tools/src/main/java/org/apache/oozie/tools/OozieSharelibCLI.java 75e932c02 
>   tools/src/test/java/org/apache/oozie/tools/OozieSharelibFileOperations.java 
> d344300ed 
>   tools/src/test/java/org/apache/oozie/tools/TestConcurrentCopyFromLocal.java 
> d77eba69d 
>   tools/src/test/java/org/apache/oozie/tools/TestCopyTaskCallable.java 
> bce0433c6 
>   tools/src/test/java/org/apache/oozie/tools/TestOozieSharelibCLI.java 
> 5929e5ca0 
>   
> tools/src/test/java/org/apache/oozie/tools/diag/IntegrationTestOozieSharelibCLI.java
>  PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/67834/diff/4/
> 
> 
> Testing
> ---
> 
> Tested manually + added integration test
> 
> 
> Thanks,
> 
> Kinga Marton
> 
>



Re: Review Request 67834: OOZIE-2829 Improve sharelib upload to accept multiple source folders

2018-07-06 Thread Kinga Marton via Review Board

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

(Updated July 6, 2018, 8:36 a.m.)


Review request for oozie, András Piros, Peter Cseh, and Peter Bacsko.


Repository: oozie-git


Description
---

Right now sharelib can be created via sharelib create -fs FS_URI -locallib 
SHARED_LIBRARY where the SHARED_LIBRARY can be a tarbal or a folder.
It would be nice to have the possibility to define additional folders to be 
uploaded into the sharelib, so the users don't have to copy or link the files 
together on their machine.
The syntax could be something like -additional-lib 
sharelibName=/path/to/source/;/path/to/some/file,sharelibName2=/path/to/some/folder


Diffs (updated)
-

  docs/src/site/twiki/AG_Install.twiki 46363a3b3 
  tools/src/main/java/org/apache/oozie/tools/OozieSharelibCLI.java 75e932c02 
  tools/src/test/java/org/apache/oozie/tools/OozieSharelibFileOperations.java 
d344300ed 
  tools/src/test/java/org/apache/oozie/tools/TestConcurrentCopyFromLocal.java 
d77eba69d 
  tools/src/test/java/org/apache/oozie/tools/TestCopyTaskCallable.java 
bce0433c6 
  tools/src/test/java/org/apache/oozie/tools/TestOozieSharelibCLI.java 
5929e5ca0 
  
tools/src/test/java/org/apache/oozie/tools/diag/IntegrationTestOozieSharelibCLI.java
 PRE-CREATION 


Diff: https://reviews.apache.org/r/67834/diff/2/

Changes: https://reviews.apache.org/r/67834/diff/1-2/


Testing
---

Tested manually + added integration test


Thanks,

Kinga Marton



Review Request 67834: OOZIE-2829 Improve sharelib upload to accept multiple source folders

2018-07-05 Thread Kinga Marton via Review Board

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

Review request for oozie, András Piros, Peter Cseh, and Peter Bacsko.


Repository: oozie-git


Description
---

Right now sharelib can be created via sharelib create -fs FS_URI -locallib 
SHARED_LIBRARY where the SHARED_LIBRARY can be a tarbal or a folder.
It would be nice to have the possibility to define additional folders to be 
uploaded into the sharelib, so the users don't have to copy or link the files 
together on their machine.
The syntax could be something like -additional-lib 
sharelibName=/path/to/source/;/path/to/some/file,sharelibName2=/path/to/some/folder


Diffs
-

  docs/src/site/twiki/AG_Install.twiki 46363a3b3 
  tools/src/main/java/org/apache/oozie/tools/OozieSharelibCLI.java 75e932c02 
  tools/src/test/java/org/apache/oozie/tools/OozieSharelibFileOperations.java 
d344300ed 
  tools/src/test/java/org/apache/oozie/tools/TestConcurrentCopyFromLocal.java 
d77eba69d 
  tools/src/test/java/org/apache/oozie/tools/TestCopyTaskCallable.java 
bce0433c6 
  tools/src/test/java/org/apache/oozie/tools/TestOozieSharelibCLI.java 
5929e5ca0 
  
tools/src/test/java/org/apache/oozie/tools/diag/IntegrationTestOozieSharelibCLI.java
 PRE-CREATION 


Diff: https://reviews.apache.org/r/67834/diff/1/


Testing
---

Tested manually + added integration test


Thanks,

Kinga Marton



Re: Review Request 66930: OOZIE-2791 ShareLib installation may fail on busy Hadoop clusters

2018-07-03 Thread Kinga Marton via Review Board

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

(Updated July 3, 2018, 3:24 p.m.)


Review request for oozie, András Piros and Peter Cseh.


Repository: oozie-git


Description
---

On a busy Hadoop cluster it can happen that users cannot install properly 
Oozie ShareLib.


Diffs (updated)
-

  tools/src/main/java/org/apache/oozie/tools/OozieSharelibCLI.java dce1c559 
  tools/src/test/java/org/apache/oozie/tools/OozieSharelibFileOperations.java 
PRE-CREATION 
  tools/src/test/java/org/apache/oozie/tools/TestBlockSizeCalculator.java 
PRE-CREATION 
  tools/src/test/java/org/apache/oozie/tools/TestConcurrentCopyFromLocal.java 
PRE-CREATION 
  tools/src/test/java/org/apache/oozie/tools/TestCopyTaskCallable.java 
PRE-CREATION 
  tools/src/test/java/org/apache/oozie/tools/TestOozieSharelibCLI.java f53d987c 


Diff: https://reviews.apache.org/r/66930/diff/8/

Changes: https://reviews.apache.org/r/66930/diff/7-8/


Testing
---

Tested manually


Thanks,

Kinga Marton



Re: Review Request 66930: OOZIE-2791 ShareLib installation may fail on busy Hadoop clusters

2018-07-03 Thread Kinga Marton via Review Board

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

(Updated July 3, 2018, 8:12 a.m.)


Review request for oozie, András Piros and Peter Cseh.


Changes
---

review fix


Repository: oozie-git


Description
---

On a busy Hadoop cluster it can happen that users cannot install properly 
Oozie ShareLib.


Diffs (updated)
-

  tools/src/main/java/org/apache/oozie/tools/OozieSharelibCLI.java dce1c559 
  tools/src/test/java/org/apache/oozie/tools/OozieSharelibFileOperations.java 
PRE-CREATION 
  tools/src/test/java/org/apache/oozie/tools/TestBlockSizeCalculator.java 
PRE-CREATION 
  tools/src/test/java/org/apache/oozie/tools/TestConcurrentCopyFromLocal.java 
PRE-CREATION 
  tools/src/test/java/org/apache/oozie/tools/TestCopyTaskCallable.java 
PRE-CREATION 
  tools/src/test/java/org/apache/oozie/tools/TestOozieSharelibCLI.java f53d987c 


Diff: https://reviews.apache.org/r/66930/diff/7/

Changes: https://reviews.apache.org/r/66930/diff/6-7/


Testing
---

Tested manually


Thanks,

Kinga Marton



Re: Review Request 66930: OOZIE-2791 ShareLib installation may fail on busy Hadoop clusters

2018-07-03 Thread Kinga Marton via Review Board


> On July 2, 2018, 2:01 p.m., Peter Bacsko wrote:
> > tools/src/main/java/org/apache/oozie/tools/OozieSharelibCLI.java
> > Lines 256-259 (patched)
> > 
> >
> > To me this looks weird. The class is named "configuration", which is 
> > telling me that it holds data. But it doesn't just hold data, it also 
> > performs an FS operation.

You are right, I have moved it to ConcurrentCopyFromLocal


- Kinga


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


On June 18, 2018, 10:03 a.m., Kinga Marton wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66930/
> ---
> 
> (Updated June 18, 2018, 10:03 a.m.)
> 
> 
> Review request for oozie, András Piros and Peter Cseh.
> 
> 
> Repository: oozie-git
> 
> 
> Description
> ---
> 
> On a busy Hadoop cluster it can happen that users cannot install properly 
> Oozie ShareLib.
> 
> 
> Diffs
> -
> 
>   tools/src/main/java/org/apache/oozie/tools/OozieSharelibCLI.java dce1c559 
>   tools/src/test/java/org/apache/oozie/tools/OozieSharelibFileOperations.java 
> PRE-CREATION 
>   tools/src/test/java/org/apache/oozie/tools/TestBlockSizeCalculator.java 
> PRE-CREATION 
>   tools/src/test/java/org/apache/oozie/tools/TestConcurrentCopyFromLocal.java 
> PRE-CREATION 
>   tools/src/test/java/org/apache/oozie/tools/TestCopyTaskCallable.java 
> PRE-CREATION 
>   tools/src/test/java/org/apache/oozie/tools/TestOozieSharelibCLI.java 
> ccad273b 
> 
> 
> Diff: https://reviews.apache.org/r/66930/diff/6/
> 
> 
> Testing
> ---
> 
> Tested manually
> 
> 
> Thanks,
> 
> Kinga Marton
> 
>



Re: Review Request 67766: OOZIE-2955 Fix Findbugs warnings related to reliance on default encoding in oozie-client

2018-07-02 Thread Kinga Marton via Review Board

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

(Updated July 2, 2018, 8:06 a.m.)


Review request for oozie and András Piros.


Repository: oozie-git


Description
---

Currently Findbugs complains about some warnings related to the reliance on the 
default encoding in the oozie-client module:
They should be fixed to get the code more reliable.


Diffs (updated)
-

  client/src/main/java/org/apache/oozie/cli/CLIParser.java bbdb80356 
  client/src/main/java/org/apache/oozie/cli/OozieCLI.java bc234e318 
  client/src/main/java/org/apache/oozie/client/AuthOozieClient.java 98f421f32 
  client/src/main/java/org/apache/oozie/client/OozieClient.java 2cc169230 
  client/src/main/java/org/apache/oozie/client/XOozieClient.java ba60f8e75 


Diff: https://reviews.apache.org/r/67766/diff/5/

Changes: https://reviews.apache.org/r/67766/diff/4-5/


Testing
---


Thanks,

Kinga Marton



Re: Review Request 67763: OOZIE-2956 Fix Findbugs warnings related to reliance on default encoding in oozie-core

2018-07-01 Thread Kinga Marton via Review Board

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

(Updated July 1, 2018, 10:47 a.m.)


Review request for oozie and András Piros.


Changes
---

findbugs fix


Repository: oozie-git


Description
---

Currently Findbugs complains about the a few warnings related to the reliance 
on the default encoding in the oozie-core module
They should be fixed to get the code more reliable.


Diffs (updated)
-

  core/pom.xml a5a776ca6 
  core/src/main/java/org/apache/oozie/StringBlob.java 6c776011c 
  core/src/main/java/org/apache/oozie/action/ActionExecutor.java 919509d35 
  core/src/main/java/org/apache/oozie/action/hadoop/LauncherHelper.java 
21c9b7e4e 
  
core/src/main/java/org/apache/oozie/action/hadoop/MapReduceActionExecutor.java 
d0b807423 
  core/src/main/java/org/apache/oozie/action/hadoop/SqoopActionExecutor.java 
69d5e7e9b 
  core/src/main/java/org/apache/oozie/action/ssh/SshActionExecutor.java 
128feee72 
  core/src/main/java/org/apache/oozie/command/bundle/BundleSubmitXCommand.java 
80e7d5d4b 
  core/src/main/java/org/apache/oozie/command/coord/CoordSubmitXCommand.java 
8bfa634f8 
  core/src/main/java/org/apache/oozie/command/coord/CoordUpdateXCommand.java 
502a800fb 
  core/src/main/java/org/apache/oozie/service/AuthorizationService.java 
251838ce7 
  core/src/main/java/org/apache/oozie/service/WorkflowAppService.java c725f493e 
  core/src/main/java/org/apache/oozie/servlet/V1JobServlet.java 361141b0d 
  core/src/main/java/org/apache/oozie/servlet/V2ValidateServlet.java 36a9de22f 
  core/src/main/java/org/apache/oozie/util/AuthUrlClient.java 4fc8f5726 
  core/src/main/java/org/apache/oozie/util/IOUtils.java 3674dc48b 
  core/src/main/java/org/apache/oozie/util/MultiFileReader.java 1ab5a7af2 
  core/src/main/java/org/apache/oozie/util/XConfiguration.java e3591db32 
  core/src/main/java/org/apache/oozie/util/XmlUtils.java 9db46b365 


Diff: https://reviews.apache.org/r/67763/diff/2/

Changes: https://reviews.apache.org/r/67763/diff/1-2/


Testing
---


Thanks,

Kinga Marton



Re: Review Request 67766: OOZIE-2955 Fix Findbugs warnings related to reliance on default encoding in oozie-client

2018-07-01 Thread Kinga Marton via Review Board

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

(Updated July 1, 2018, 10:19 a.m.)


Review request for oozie and András Piros.


Changes
---

findbug fix


Repository: oozie-git


Description
---

Currently Findbugs complains about some warnings related to the reliance on the 
default encoding in the oozie-client module:
They should be fixed to get the code more reliable.


Diffs (updated)
-

  client/src/main/java/org/apache/oozie/cli/CLIParser.java bbdb80356 
  client/src/main/java/org/apache/oozie/cli/OozieCLI.java bc234e318 
  client/src/main/java/org/apache/oozie/client/AuthOozieClient.java 98f421f32 
  client/src/main/java/org/apache/oozie/client/OozieClient.java 2cc169230 
  client/src/main/java/org/apache/oozie/client/XOozieClient.java ba60f8e75 


Diff: https://reviews.apache.org/r/67766/diff/4/

Changes: https://reviews.apache.org/r/67766/diff/3-4/


Testing
---


Thanks,

Kinga Marton



Re: Review Request 67766: OOZIE-2955 Fix Findbugs warnings related to reliance on default encoding in oozie-client

2018-07-01 Thread Kinga Marton via Review Board

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

(Updated July 1, 2018, 10:02 a.m.)


Review request for oozie and András Piros.


Repository: oozie-git


Description
---

Currently Findbugs complains about some warnings related to the reliance on the 
default encoding in the oozie-client module:
They should be fixed to get the code more reliable.


Diffs (updated)
-

  client/src/main/java/org/apache/oozie/cli/CLIParser.java bbdb80356 
  client/src/main/java/org/apache/oozie/cli/OozieCLI.java bc234e318 
  client/src/main/java/org/apache/oozie/client/AuthOozieClient.java 98f421f32 
  client/src/main/java/org/apache/oozie/client/OozieClient.java 2cc169230 
  client/src/main/java/org/apache/oozie/client/XOozieClient.java ba60f8e75 


Diff: https://reviews.apache.org/r/67766/diff/3/

Changes: https://reviews.apache.org/r/67766/diff/2-3/


Testing
---


Thanks,

Kinga Marton



Re: Review Request 67766: OOZIE-2955 Fix Findbugs warnings related to reliance on default encoding in oozie-client

2018-06-28 Thread Kinga Marton via Review Board

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

(Updated June 28, 2018, 8:58 a.m.)


Review request for oozie and András Piros.


Repository: oozie-git


Description
---

Currently Findbugs complains about some warnings related to the reliance on the 
default encoding in the oozie-client module:
They should be fixed to get the code more reliable.


Diffs (updated)
-

  client/src/main/java/org/apache/oozie/cli/CLIParser.java bbdb8035 
  client/src/main/java/org/apache/oozie/cli/OozieCLI.java bc234e31 
  client/src/main/java/org/apache/oozie/client/AuthOozieClient.java 98f421f3 
  client/src/main/java/org/apache/oozie/client/OozieClient.java 2cc16923 
  client/src/main/java/org/apache/oozie/client/XOozieClient.java ba60f8e7 


Diff: https://reviews.apache.org/r/67766/diff/2/

Changes: https://reviews.apache.org/r/67766/diff/1-2/


Testing
---


Thanks,

Kinga Marton



Review Request 67766: OOZIE-2955 Fix Findbugs warnings related to reliance on default encoding in oozie-client

2018-06-28 Thread Kinga Marton via Review Board

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

Review request for oozie and András Piros.


Repository: oozie-git


Description
---

Currently Findbugs complains about some warnings related to the reliance on the 
default encoding in the oozie-client module:
They should be fixed to get the code more reliable.


Diffs
-

  client/src/main/java/org/apache/oozie/cli/CLIParser.java bbdb8035 
  client/src/main/java/org/apache/oozie/cli/OozieCLI.java bc234e31 
  client/src/main/java/org/apache/oozie/client/AuthOozieClient.java 98f421f3 
  client/src/main/java/org/apache/oozie/client/OozieClient.java 2cc16923 
  client/src/main/java/org/apache/oozie/client/XOozieClient.java ba60f8e7 


Diff: https://reviews.apache.org/r/67766/diff/1/


Testing
---


Thanks,

Kinga Marton



Review Request 67763: OOZIE-2956 Fix Findbugs warnings related to reliance on default encoding in oozie-core

2018-06-27 Thread Kinga Marton via Review Board

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

Review request for oozie and András Piros.


Repository: oozie-git


Description
---

Currently Findbugs complains about the a few warnings related to the reliance 
on the default encoding in the oozie-core module
They should be fixed to get the code more reliable.


Diffs
-

  core/src/main/java/org/apache/oozie/StringBlob.java 6c776011 
  core/src/main/java/org/apache/oozie/action/ActionExecutor.java 919509d3 
  core/src/main/java/org/apache/oozie/action/hadoop/LauncherHelper.java 
21c9b7e4 
  
core/src/main/java/org/apache/oozie/action/hadoop/MapReduceActionExecutor.java 
d0b80742 
  core/src/main/java/org/apache/oozie/action/hadoop/SqoopActionExecutor.java 
69d5e7e9 
  core/src/main/java/org/apache/oozie/action/ssh/SshActionExecutor.java 
128feee7 
  core/src/main/java/org/apache/oozie/command/bundle/BundleSubmitXCommand.java 
80e7d5d4 
  core/src/main/java/org/apache/oozie/command/coord/CoordSubmitXCommand.java 
8bfa634f 
  core/src/main/java/org/apache/oozie/command/coord/CoordUpdateXCommand.java 
502a800f 
  core/src/main/java/org/apache/oozie/service/AuthorizationService.java 
251838ce 
  core/src/main/java/org/apache/oozie/service/WorkflowAppService.java c725f493 
  core/src/main/java/org/apache/oozie/servlet/V1JobServlet.java 361141b0 
  core/src/main/java/org/apache/oozie/servlet/V2ValidateServlet.java 36a9de22 
  core/src/main/java/org/apache/oozie/util/AuthUrlClient.java 4fc8f572 
  core/src/main/java/org/apache/oozie/util/IOUtils.java 3674dc48 
  core/src/main/java/org/apache/oozie/util/MultiFileReader.java 1ab5a7af 
  core/src/main/java/org/apache/oozie/util/XConfiguration.java e3591db3 
  core/src/main/java/org/apache/oozie/util/XmlUtils.java 9db46b36 


Diff: https://reviews.apache.org/r/67763/diff/1/


Testing
---


Thanks,

Kinga Marton



Re: Review Request 66930: OOZIE-2791 ShareLib installation may fail on busy Hadoop clusters

2018-06-18 Thread Kinga Marton via Review Board

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

(Updated June 18, 2018, 10:03 a.m.)


Review request for oozie, András Piros and Peter Cseh.


Repository: oozie-git


Description
---

On a busy Hadoop cluster it can happen that users cannot install properly 
Oozie ShareLib.


Diffs (updated)
-

  tools/src/main/java/org/apache/oozie/tools/OozieSharelibCLI.java dce1c559 
  tools/src/test/java/org/apache/oozie/tools/OozieSharelibFileOperations.java 
PRE-CREATION 
  tools/src/test/java/org/apache/oozie/tools/TestBlockSizeCalculator.java 
PRE-CREATION 
  tools/src/test/java/org/apache/oozie/tools/TestConcurrentCopyFromLocal.java 
PRE-CREATION 
  tools/src/test/java/org/apache/oozie/tools/TestCopyTaskCallable.java 
PRE-CREATION 
  tools/src/test/java/org/apache/oozie/tools/TestOozieSharelibCLI.java ccad273b 


Diff: https://reviews.apache.org/r/66930/diff/6/

Changes: https://reviews.apache.org/r/66930/diff/5-6/


Testing
---

Tested manually


Thanks,

Kinga Marton



Re: Review Request 66930: OOZIE-2791 ShareLib installation may fail on busy Hadoop clusters

2018-06-06 Thread Kinga Marton via Review Board

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

(Updated June 6, 2018, 12:34 p.m.)


Review request for oozie, András Piros and Peter Cseh.


Changes
---

review fix


Repository: oozie-git


Description
---

On a busy Hadoop cluster it can happen that users cannot install properly 
Oozie ShareLib.


Diffs (updated)
-

  tools/src/main/java/org/apache/oozie/tools/OozieSharelibCLI.java dce1c559 
  tools/src/test/java/org/apache/oozie/tools/TestBlockSizeCalculator.java 
PRE-CREATION 
  tools/src/test/java/org/apache/oozie/tools/TestConcurrentCopyFromLocal.java 
PRE-CREATION 
  tools/src/test/java/org/apache/oozie/tools/TestCopyTaskCallable.java 
PRE-CREATION 
  tools/src/test/java/org/apache/oozie/tools/TestOozieSharelibCLI.java ccad273b 
  tools/src/test/java/org/apache/oozie/tools/TestOozieSharelibCLIHelper.java 
PRE-CREATION 


Diff: https://reviews.apache.org/r/66930/diff/5/

Changes: https://reviews.apache.org/r/66930/diff/4-5/


Testing
---

Tested manually


Thanks,

Kinga Marton



Re: Review Request 66930: OOZIE-2791 ShareLib installation may fail on busy Hadoop clusters

2018-06-05 Thread Kinga Marton via Review Board

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

(Updated June 5, 2018, 8:37 a.m.)


Review request for oozie, András Piros and Peter Cseh.


Changes
---

findbug fix


Repository: oozie-git


Description
---

On a busy Hadoop cluster it can happen that users cannot install properly 
Oozie ShareLib.


Diffs (updated)
-

  tools/src/main/java/org/apache/oozie/tools/OozieSharelibCLI.java dce1c559 
  tools/src/test/java/org/apache/oozie/tools/TestBlockSizeCalculator.java 
PRE-CREATION 
  tools/src/test/java/org/apache/oozie/tools/TestConcurrentCopyFromLocal.java 
PRE-CREATION 
  tools/src/test/java/org/apache/oozie/tools/TestCopyTaskCallable.java 
PRE-CREATION 
  tools/src/test/java/org/apache/oozie/tools/TestOozieSharelibCLI.java ccad273b 


Diff: https://reviews.apache.org/r/66930/diff/4/

Changes: https://reviews.apache.org/r/66930/diff/3-4/


Testing
---

Tested manually


Thanks,

Kinga Marton



Re: Review Request 66930: OOZIE-2791 ShareLib installation may fail on busy Hadoop clusters

2018-06-04 Thread Kinga Marton via Review Board

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

(Updated June 4, 2018, 1:55 p.m.)


Review request for oozie, András Piros and Peter Cseh.


Changes
---

review fix + test classes for CopyTaskCallable and ConcurrentCopyFromLocal


Repository: oozie-git


Description
---

On a busy Hadoop cluster it can happen that users cannot install properly 
Oozie ShareLib.


Diffs (updated)
-

  tools/src/main/java/org/apache/oozie/tools/OozieSharelibCLI.java dce1c559 
  tools/src/test/java/org/apache/oozie/tools/TestBlockSizeCalculator.java 
PRE-CREATION 
  tools/src/test/java/org/apache/oozie/tools/TestConcurrentCopyFromLocal.java 
PRE-CREATION 
  tools/src/test/java/org/apache/oozie/tools/TestCopyTaskCallable.java 
PRE-CREATION 
  tools/src/test/java/org/apache/oozie/tools/TestOozieSharelibCLI.java ccad273b 


Diff: https://reviews.apache.org/r/66930/diff/3/

Changes: https://reviews.apache.org/r/66930/diff/2-3/


Testing
---

Tested manually


Thanks,

Kinga Marton



Re: Review Request 66930: OOZIE-2791 ShareLib installation may fail on busy Hadoop clusters

2018-05-10 Thread Kinga Marton via Review Board


> On May 3, 2018, 3:33 p.m., András Piros wrote:
> > tools/src/main/java/org/apache/oozie/tools/OozieSharelibCLI.java
> > Lines 224-235 (patched)
> > 
> >
> > Please extract to another `static final class`, and add integration 
> > tests that use `XTestCase` to check copying real data onto `MiniDFSCluster`.

TestOozieSharelibCLI:testOozieSharelibCLICreateConcurrent() is testing if the 
files are copied in concurrent mode.


> On May 3, 2018, 3:33 p.m., András Piros wrote:
> > tools/src/main/java/org/apache/oozie/tools/OozieSharelibCLI.java
> > Lines 322 (patched)
> > 
> >
> > I'm just wondering whether in case any errors we should 1) rollback the 
> > stuff we halfway managed to copy 2) die after this `System.err.println()` 
> > call.

1. The possibility of a "partial sharelib update" is handled with OOZIE-2629.
2. In case of an Exception the exit status will be 1. This is handled at the 
end of the run() method.


> On May 3, 2018, 3:33 p.m., András Piros wrote:
> > tools/src/main/java/org/apache/oozie/tools/OozieSharelibCLI.java
> > Lines 349-351 (patched)
> > 
> >
> > I'm just wondering whether in case any errors we should 1) rollback the 
> > stuff we halfway managed to copy 2) die after this `System.err.println()` 
> > call.

1. The possibility of a "partial sharelib update" is handled with OOZIE-2629.
2. In case of an Exception the exit status will be 1. This is handled at the 
end of the run() method.


- Kinga


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


On May 10, 2018, 7:59 a.m., Kinga Marton wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66930/
> ---
> 
> (Updated May 10, 2018, 7:59 a.m.)
> 
> 
> Review request for oozie, András Piros and Peter Cseh.
> 
> 
> Repository: oozie-git
> 
> 
> Description
> ---
> 
> On a busy Hadoop cluster it can happen that users cannot install properly 
> Oozie ShareLib.
> 
> 
> Diffs
> -
> 
>   tools/src/main/java/org/apache/oozie/tools/OozieSharelibCLI.java dce1c559 
>   tools/src/test/java/org/apache/oozie/tools/TestBlockSizeCalculator.java 
> PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/66930/diff/2/
> 
> 
> Testing
> ---
> 
> Tested manually
> 
> 
> Thanks,
> 
> Kinga Marton
> 
>



Re: Review Request 66930: OOZIE-2791 ShareLib installation may fail on busy Hadoop clusters

2018-05-10 Thread Kinga Marton via Review Board

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

(Updated May 10, 2018, 7:59 a.m.)


Review request for oozie, András Piros and Peter Cseh.


Changes
---

review fix


Repository: oozie-git


Description
---

On a busy Hadoop cluster it can happen that users cannot install properly 
Oozie ShareLib.


Diffs (updated)
-

  tools/src/main/java/org/apache/oozie/tools/OozieSharelibCLI.java dce1c559 
  tools/src/test/java/org/apache/oozie/tools/TestBlockSizeCalculator.java 
PRE-CREATION 


Diff: https://reviews.apache.org/r/66930/diff/2/

Changes: https://reviews.apache.org/r/66930/diff/1-2/


Testing
---

Tested manually


Thanks,

Kinga Marton



Review Request 66930: OOZIE-2791 ShareLib installation may fail on busy Hadoop clusters

2018-05-03 Thread Kinga Marton via Review Board

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

Review request for oozie, András Piros and Peter Cseh.


Repository: oozie-git


Description
---

On a busy Hadoop cluster it can happen that users cannot install properly 
Oozie ShareLib.


Diffs
-

  tools/src/main/java/org/apache/oozie/tools/OozieSharelibCLI.java dce1c559b 


Diff: https://reviews.apache.org/r/66930/diff/1/


Testing
---

Tested manually


Thanks,

Kinga Marton



Re: Review Request 65481: OOZIE-3134 - Potential inconsistency between the in-memory SLA map and the Oozie database

2018-03-23 Thread Kinga Marton via Review Board

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

(Updated March 23, 2018, 7:28 a.m.)


Review request for oozie, András Piros and Attila Sasvari.


Repository: oozie-git


Description
---

Upon SLACalculatorMemory.addRegistration, Oozie puts an entry into an in-memory 
concurrent hashmap ("slaMap"), and later persists it in the Oozie database.
However, if there is a failure during the database operation, a 
JPAExecutorException is thrown, and the entry is not removed from the SLA map.
It may introduce inconsistency between the Oozie database and the SLA map.
To prevent this, a rollback mechanism (with proper logging) should be 
implemented. It would also make sense to do more sanity/consistency check in 
the Oozie server.


Diffs (updated)
-

  core/src/main/java/org/apache/oozie/sla/SLACalculatorMemory.java ef019e73 
  
core/src/main/java/org/apache/oozie/util/db/AlwaysFailingHSQLDriverMapper.java 
PRE-CREATION 
  core/src/main/java/org/apache/oozie/util/db/FailingConnectionWrapper.java 
0e310253 
  core/src/main/java/org/apache/oozie/util/db/FailingDBHelperForTest.java 
PRE-CREATION 
  core/src/main/java/org/apache/oozie/util/db/FailingHSQLDBDriverWrapper.java 
fe9f08b1 
  core/src/main/java/org/apache/oozie/util/db/FailingMySQLDriverWrapper.java 
f0e2b181 
  core/src/test/java/org/apache/oozie/sla/TestSLACalculatorMemory.java ee906f45 


Diff: https://reviews.apache.org/r/65481/diff/6/

Changes: https://reviews.apache.org/r/65481/diff/5-6/


Testing
---


Thanks,

Kinga Marton



Re: Review Request 65481: OOZIE-3134 - Potential inconsistency between the in-memory SLA map and the Oozie database

2018-03-22 Thread Kinga Marton via Review Board

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

(Updated March 22, 2018, 4:44 p.m.)


Review request for oozie, András Piros and Attila Sasvari.


Repository: oozie-git


Description
---

Upon SLACalculatorMemory.addRegistration, Oozie puts an entry into an in-memory 
concurrent hashmap ("slaMap"), and later persists it in the Oozie database.
However, if there is a failure during the database operation, a 
JPAExecutorException is thrown, and the entry is not removed from the SLA map.
It may introduce inconsistency between the Oozie database and the SLA map.
To prevent this, a rollback mechanism (with proper logging) should be 
implemented. It would also make sense to do more sanity/consistency check in 
the Oozie server.


Diffs (updated)
-

  core/src/main/java/org/apache/oozie/sla/SLACalculatorMemory.java ef019e73 
  
core/src/main/java/org/apache/oozie/util/db/AlwaysFailingHSQLDriverMapper.java 
PRE-CREATION 
  core/src/main/java/org/apache/oozie/util/db/FailingConnectionWrapper.java 
0e310253 
  core/src/main/java/org/apache/oozie/util/db/FailingDBHelperForTest.java 
PRE-CREATION 
  core/src/main/java/org/apache/oozie/util/db/FailingHSQLDBDriverWrapper.java 
fe9f08b1 
  core/src/main/java/org/apache/oozie/util/db/FailingMySQLDriverWrapper.java 
f0e2b181 
  core/src/test/java/org/apache/oozie/sla/TestSLACalculatorMemory.java ee906f45 


Diff: https://reviews.apache.org/r/65481/diff/5/

Changes: https://reviews.apache.org/r/65481/diff/4-5/


Testing
---


Thanks,

Kinga Marton



Re: Review Request 65481: OOZIE-3134 - Potential inconsistency between the in-memory SLA map and the Oozie database

2018-02-19 Thread Kinga Marton via Review Board

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

(Updated Feb. 19, 2018, 2:14 p.m.)


Review request for oozie, András Piros and Attila Sasvari.


Repository: oozie-git


Description
---

Upon SLACalculatorMemory.addRegistration, Oozie puts an entry into an in-memory 
concurrent hashmap ("slaMap"), and later persists it in the Oozie database.
However, if there is a failure during the database operation, a 
JPAExecutorException is thrown, and the entry is not removed from the SLA map.
It may introduce inconsistency between the Oozie database and the SLA map.
To prevent this, a rollback mechanism (with proper logging) should be 
implemented. It would also make sense to do more sanity/consistency check in 
the Oozie server.


Diffs (updated)
-

  core/src/main/java/org/apache/oozie/sla/SLACalculatorMemory.java ef019e73 
  
core/src/main/java/org/apache/oozie/util/db/AlwaysFailingHSQLDriverMapper.java 
PRE-CREATION 
  core/src/main/java/org/apache/oozie/util/db/FailingConnectionWrapper.java 
0e310253 
  core/src/main/java/org/apache/oozie/util/db/FailingDBHelperForTest.java 
PRE-CREATION 
  core/src/main/java/org/apache/oozie/util/db/FailingHSQLDBDriverWrapper.java 
fe9f08b1 
  core/src/main/java/org/apache/oozie/util/db/FailingMySQLDriverWrapper.java 
f0e2b181 
  core/src/test/java/org/apache/oozie/sla/TestSLACalculatorMemory.java ee906f45 


Diff: https://reviews.apache.org/r/65481/diff/4/

Changes: https://reviews.apache.org/r/65481/diff/3-4/


Testing
---


Thanks,

Kinga Marton



Re: Review Request 65481: OOZIE-3134 - Potential inconsistency between the in-memory SLA map and the Oozie database

2018-02-13 Thread Kinga Marton via Review Board

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

(Updated Feb. 13, 2018, 3:19 p.m.)


Review request for oozie, András Piros and Attila Sasvari.


Repository: oozie-git


Description
---

Upon SLACalculatorMemory.addRegistration, Oozie puts an entry into an in-memory 
concurrent hashmap ("slaMap"), and later persists it in the Oozie database.
However, if there is a failure during the database operation, a 
JPAExecutorException is thrown, and the entry is not removed from the SLA map.
It may introduce inconsistency between the Oozie database and the SLA map.
To prevent this, a rollback mechanism (with proper logging) should be 
implemented. It would also make sense to do more sanity/consistency check in 
the Oozie server.


Diffs (updated)
-

  core/src/main/java/org/apache/oozie/sla/SLACalculatorMemory.java ef019e73 
  
core/src/main/java/org/apache/oozie/util/db/AlwaysFailingHSQLDriverMapper.java 
PRE-CREATION 
  core/src/main/java/org/apache/oozie/util/db/FailingConnectionWrapper.java 
0e310253 
  core/src/main/java/org/apache/oozie/util/db/FailingDBHelperForTest.java 
PRE-CREATION 
  core/src/main/java/org/apache/oozie/util/db/FailingHSQLDBDriverWrapper.java 
fe9f08b1 
  core/src/main/java/org/apache/oozie/util/db/FailingMySQLDriverWrapper.java 
f0e2b181 
  core/src/test/java/org/apache/oozie/sla/TestSLACalculatorMemory.java ee906f45 


Diff: https://reviews.apache.org/r/65481/diff/3/

Changes: https://reviews.apache.org/r/65481/diff/2-3/


Testing
---


Thanks,

Kinga Marton



Re: Review Request 65481: OOZIE-3134 - Potential inconsistency between the in-memory SLA map and the Oozie database

2018-02-13 Thread Kinga Marton via Review Board


> On Feb. 6, 2018, 4:15 p.m., András Piros wrote:
> > core/src/main/java/org/apache/oozie/util/db/ThreadVariables.java
> > Lines 23-37 (patched)
> > 
> >
> > I don't see calling `setDbPredicate()` or `unsetDbPredicate()` from 
> > anywhere else than test code.
> > 
> > When using `ThreadLocal` we have to be very careful to unset after 
> > every use to avoid resource / memory leaks.
> > 
> > Generally, I think it would be a better idea to inject a new DB 
> > predicate by other means - I would only go for `ThreadLocal` usage when 
> > most of the time I need different `dbPredicate` to every `Thread` - which 
> > is not the case here.
> > 
> > Let's think a bit more on `Predicate` injecting scenarios apart from 
> > using `ThreadLocal`.

My next ideea is what Peti suggested: a helper class with a static field. Is 
not a very elegant solution. Do you have some better ideeas?


- Kinga


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


On Feb. 6, 2018, 2:43 p.m., Kinga Marton wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65481/
> ---
> 
> (Updated Feb. 6, 2018, 2:43 p.m.)
> 
> 
> Review request for oozie, András Piros and Attila Sasvari.
> 
> 
> Repository: oozie-git
> 
> 
> Description
> ---
> 
> Upon SLACalculatorMemory.addRegistration, Oozie puts an entry into an 
> in-memory concurrent hashmap ("slaMap"), and later persists it in the Oozie 
> database.
> However, if there is a failure during the database operation, a 
> JPAExecutorException is thrown, and the entry is not removed from the SLA map.
> It may introduce inconsistency between the Oozie database and the SLA map.
> To prevent this, a rollback mechanism (with proper logging) should be 
> implemented. It would also make sense to do more sanity/consistency check in 
> the Oozie server.
> 
> 
> Diffs
> -
> 
>   core/src/main/java/org/apache/oozie/sla/SLACalculatorMemory.java ef019e73 
>   
> core/src/main/java/org/apache/oozie/util/db/AlwaysFailingHSQLDriverMapper.java
>  PRE-CREATION 
>   core/src/main/java/org/apache/oozie/util/db/FailingConnectionWrapper.java 
> 0e310253 
>   core/src/main/java/org/apache/oozie/util/db/FailingHSQLDBDriverWrapper.java 
> fe9f08b1 
>   core/src/main/java/org/apache/oozie/util/db/FailingMySQLDriverWrapper.java 
> f0e2b181 
>   core/src/main/java/org/apache/oozie/util/db/ThreadVariables.java 
> PRE-CREATION 
>   core/src/test/java/org/apache/oozie/sla/TestSLACalculatorMemory.java 
> ee906f45 
> 
> 
> Diff: https://reviews.apache.org/r/65481/diff/2/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Kinga Marton
> 
>



Re: Review Request 65481: OOZIE-3134 - Potential inconsistency between the in-memory SLA map and the Oozie database

2018-02-06 Thread Kinga Marton via Review Board


> On Feb. 2, 2018, 2:09 p.m., András Piros wrote:
> > core/src/main/java/org/apache/oozie/util/db/FailingHSQLDBDriverWrapper.java
> > Lines 36-39 (patched)
> > 
> >
> > Usage of System#getProperty(String, String) would be much cleaner:
> > 
> > 
> > https://docs.oracle.com/javase/8/docs/api/java/lang/System.html#getProperty-java.lang.String-java.lang.String-

With the AlwaysFailingdriverMapper, I will not need this system property anymore


> On Feb. 2, 2018, 2:09 p.m., András Piros wrote:
> > core/src/main/java/org/apache/oozie/util/db/RuntimeExceptionInjector.java
> > Lines 46-53 (patched)
> > 
> >
> > Would extract method refreshFailurePercent(), as well use 
> > https://docs.oracle.com/javase/8/docs/api/java/lang/System.html#getProperty-java.lang.String-java.lang.String-
> > 
> > But even better, I'd rather create three of this:
> > - FailingHSQLDBDriverWrapper
> > - NeverFailingHSQLDBDriverWrapper
> > - AlwaysFailingHSQLDBDriverWrapper
> > 
> > and reset this Configuration property for each of the unit tests.

I have created the AlwaysFailingHSQLDBDriverWrapper, but I skipped the creation 
of NeverFailingHSQLDBDriverWrapper, because the defaut jdbdDriver is actually a 
"Never failing" one.


- Kinga


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


On Feb. 6, 2018, 2:43 p.m., Kinga Marton wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65481/
> ---
> 
> (Updated Feb. 6, 2018, 2:43 p.m.)
> 
> 
> Review request for oozie, András Piros and Attila Sasvari.
> 
> 
> Repository: oozie-git
> 
> 
> Description
> ---
> 
> Upon SLACalculatorMemory.addRegistration, Oozie puts an entry into an 
> in-memory concurrent hashmap ("slaMap"), and later persists it in the Oozie 
> database.
> However, if there is a failure during the database operation, a 
> JPAExecutorException is thrown, and the entry is not removed from the SLA map.
> It may introduce inconsistency between the Oozie database and the SLA map.
> To prevent this, a rollback mechanism (with proper logging) should be 
> implemented. It would also make sense to do more sanity/consistency check in 
> the Oozie server.
> 
> 
> Diffs
> -
> 
>   core/src/main/java/org/apache/oozie/sla/SLACalculatorMemory.java ef019e73 
>   
> core/src/main/java/org/apache/oozie/util/db/AlwaysFailingHSQLDriverMapper.java
>  PRE-CREATION 
>   core/src/main/java/org/apache/oozie/util/db/FailingConnectionWrapper.java 
> 0e310253 
>   core/src/main/java/org/apache/oozie/util/db/FailingHSQLDBDriverWrapper.java 
> fe9f08b1 
>   core/src/main/java/org/apache/oozie/util/db/FailingMySQLDriverWrapper.java 
> f0e2b181 
>   core/src/main/java/org/apache/oozie/util/db/ThreadVariables.java 
> PRE-CREATION 
>   core/src/test/java/org/apache/oozie/sla/TestSLACalculatorMemory.java 
> ee906f45 
> 
> 
> Diff: https://reviews.apache.org/r/65481/diff/2/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Kinga Marton
> 
>



Re: Review Request 65481: OOZIE-3134 - Potential inconsistency between the in-memory SLA map and the Oozie database

2018-02-06 Thread Kinga Marton via Review Board

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

(Updated Feb. 6, 2018, 2:43 p.m.)


Review request for oozie, András Piros and Attila Sasvari.


Repository: oozie-git


Description
---

Upon SLACalculatorMemory.addRegistration, Oozie puts an entry into an in-memory 
concurrent hashmap ("slaMap"), and later persists it in the Oozie database.
However, if there is a failure during the database operation, a 
JPAExecutorException is thrown, and the entry is not removed from the SLA map.
It may introduce inconsistency between the Oozie database and the SLA map.
To prevent this, a rollback mechanism (with proper logging) should be 
implemented. It would also make sense to do more sanity/consistency check in 
the Oozie server.


Diffs (updated)
-

  core/src/main/java/org/apache/oozie/sla/SLACalculatorMemory.java ef019e73 
  
core/src/main/java/org/apache/oozie/util/db/AlwaysFailingHSQLDriverMapper.java 
PRE-CREATION 
  core/src/main/java/org/apache/oozie/util/db/FailingConnectionWrapper.java 
0e310253 
  core/src/main/java/org/apache/oozie/util/db/FailingHSQLDBDriverWrapper.java 
fe9f08b1 
  core/src/main/java/org/apache/oozie/util/db/FailingMySQLDriverWrapper.java 
f0e2b181 
  core/src/main/java/org/apache/oozie/util/db/ThreadVariables.java PRE-CREATION 
  core/src/test/java/org/apache/oozie/sla/TestSLACalculatorMemory.java ee906f45 


Diff: https://reviews.apache.org/r/65481/diff/2/

Changes: https://reviews.apache.org/r/65481/diff/1-2/


Testing
---


Thanks,

Kinga Marton



Review Request 65481: OOZIE-3134 - Potential inconsistency between the in-memory SLA map and the Oozie database

2018-02-02 Thread Kinga Marton via Review Board

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

Review request for oozie, András Piros and Attila Sasvari.


Repository: oozie-git


Description
---

Upon SLACalculatorMemory.addRegistration, Oozie puts an entry into an in-memory 
concurrent hashmap ("slaMap"), and later persists it in the Oozie database.
However, if there is a failure during the database operation, a 
JPAExecutorException is thrown, and the entry is not removed from the SLA map.
It may introduce inconsistency between the Oozie database and the SLA map.
To prevent this, a rollback mechanism (with proper logging) should be 
implemented. It would also make sense to do more sanity/consistency check in 
the Oozie server.


Diffs
-

  core/src/main/java/org/apache/oozie/sla/SLACalculatorMemory.java ef019e73 
  core/src/main/java/org/apache/oozie/util/db/FailingConnectionWrapper.java 
0e310253 
  core/src/main/java/org/apache/oozie/util/db/FailingHSQLDBDriverWrapper.java 
fe9f08b1 
  core/src/main/java/org/apache/oozie/util/db/FailingMySQLDriverWrapper.java 
f0e2b181 
  core/src/main/java/org/apache/oozie/util/db/RuntimeExceptionInjector.java 
2b55e858 
  core/src/test/java/org/apache/oozie/service/TestConfigurationService.java 
1b68090d 
  core/src/test/java/org/apache/oozie/sla/TestSLACalculatorMemory.java ee906f45 
  core/src/test/resources/hsqldb-oozie-site.xml fa5fe9c3 


Diff: https://reviews.apache.org/r/65481/diff/1/


Testing
---


Thanks,

Kinga Marton



***UNCHECKED*** Re: Review Request 65287: OOZIE-3157 - Setup truststore so that it also works in HTTP only mode

2018-01-26 Thread Kinga Marton via Review Board

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

(Updated Jan. 26, 2018, 1:16 p.m.)


Review request for oozie, András Piros, Attila Sasvari, and Peter Cseh.


Repository: oozie-git


Description
---

oozie.https.truststore.file is not read and used when oozie.https.enabled is 
false in oozie-site xml. As a result, the Oozie server will be unable to 
communicate with servers with unsigned certificate. It is a critical problem as 
authentication may involve external servers (for example KMS with self-signed 
certificate). Submitting a workflow in such an environment can result in an 
exception like:

`2018-01-08 10:13:51,471 WARN 
org.apache.hadoop.crypto.key.kms.LoadBalancingKMSClientProvider: 
SERVER[myserver] KMS provider at [https://myserver:16000/kms/v1/] threw an 
IOException: 
javax.net.ssl.SSLHandshakeException: sun.security.validator.ValidatorException: 
PKIX path building failed: 
sun.security.provider.certpath.SunCertPathBuilderException: unable to
 find valid certification path to requested target
at sun.security.ssl.Alerts.getSSLException(Alerts.java:192)
at sun.security.ssl.SSLSocketImpl.fatal(SSLSocketImpl.java:1959)
at sun.security.ssl.Handshaker.fatalSE(Handshaker.java:302)
at sun.security.ssl.Handshaker.fatalSE(Handshaker.java:296)
at 
sun.security.ssl.ClientHandshaker.serverCertificate(ClientHandshaker.java:1514)
at 
sun.security.ssl.ClientHandshaker.processMessage(ClientHandshaker.java:216)
at sun.security.ssl.Handshaker.processLoop(Handshaker.java:1026)
at sun.security.ssl.Handshaker.process_record(Handshaker.java:961)
at sun.security.ssl.SSLSocketImpl.readRecord(SSLSocketImpl.java:1072)
at 
sun.security.ssl.SSLSocketImpl.performInitialHandshake(SSLSocketImpl.java:1385)
at 
sun.security.ssl.SSLSocketImpl.startHandshake(SSLSocketImpl.java:1413)
at 
sun.security.ssl.SSLSocketImpl.startHandshake(SSLSocketImpl.java:1397)
at 
sun.net.www.protocol.https.HttpsClient.afterConnect(HttpsClient.java:559)
at 
sun.net.www.protocol.https.AbstractDelegateHttpsURLConnection.connect(AbstractDelegateHttpsURLConnection.java:185)
at 
sun.net.www.protocol.https.HttpsURLConnectionImpl.connect(HttpsURLConnectionImpl.java:153)
at 
org.apache.hadoop.security.authentication.client.KerberosAuthenticator.authenticate(KerberosAuthenticator.java:186)
at 
org.apache.hadoop.security.token.delegation.web.DelegationTokenAuthenticator.authenticate(DelegationTokenAuthenticator.java:144)
at 
org.apache.hadoop.security.authentication.client.AuthenticatedURL.openConnection(AuthenticatedURL.java:348)
at 
org.apache.hadoop.security.token.delegation.web.DelegationTokenAuthenticatedURL.openConnection(DelegationTokenAuthenticatedURL.java:333)
at 
org.apache.hadoop.crypto.key.kms.KMSClientProvider$1.run(KMSClientProvider.java:477)
at 
org.apache.hadoop.crypto.key.kms.KMSClientProvider$1.run(KMSClientProvider.java:472)
at java.security.AccessController.doPrivileged(Native Method)
at javax.security.auth.Subject.doAs(Subject.java:422)
at 
org.apache.hadoop.security.UserGroupInformation.doAs(UserGroupInformation.java:1962)
at 
org.apache.hadoop.crypto.key.kms.KMSClientProvider.createConnection(KMSClientProvider.java:471)
at 
org.apache.hadoop.crypto.key.kms.KMSClientProvider.decryptEncryptedKey(KMSClientProvider.java:776)
at 
org.apache.hadoop.crypto.key.kms.LoadBalancingKMSClientProvider$5.call(LoadBalancingKMSClientProvider.java:287)
at 
org.apache.hadoop.crypto.key.kms.LoadBalancingKMSClientProvider$5.call(LoadBalancingKMSClientProvider.java:283)
at 
org.apache.hadoop.crypto.key.kms.LoadBalancingKMSClientProvider.doOp(LoadBalancingKMSClientProvider.java:123)
at 
org.apache.hadoop.crypto.key.kms.LoadBalancingKMSClientProvider.decryptEncryptedKey(LoadBalancingKMSClientProvider.java:283)
at 
org.apache.hadoop.crypto.key.KeyProviderCryptoExtension.decryptEncryptedKey(KeyProviderCryptoExtension.java:532)
at 
org.apache.hadoop.hdfs.DFSClient.decryptEncryptedDataEncryptionKey(DFSClient.java:926)
at 
org.apache.hadoop.hdfs.DFSClient.createWrappedInputStream(DFSClient.java:945)
at 
org.apache.hadoop.hdfs.DistributedFileSystem$4.doCall(DistributedFileSystem.java:315)
at 
org.apache.hadoop.hdfs.DistributedFileSystem$4.doCall(DistributedFileSystem.java:310)
at 
org.apache.hadoop.fs.FileSystemLinkResolver.resolve(FileSystemLinkResolver.java:81)
at 
org.apache.hadoop.hdfs.DistributedFileSystem.open(DistributedFileSystem.java:322)
at org.apache.hadoop.fs.FileSystem.open(FileSystem.java:949)
at 

Re: Review Request 65287: OOZIE-3157 - Setup truststore so that it also works in HTTP only mode

2018-01-26 Thread Kinga Marton via Review Board

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

(Updated Jan. 26, 2018, 1:13 p.m.)


Review request for oozie, András Piros, Attila Sasvari, and Peter Cseh.


Repository: oozie-git


Description
---

oozie.https.truststore.file is not read and used when oozie.https.enabled is 
false in oozie-site xml. As a result, the Oozie server will be unable to 
communicate with servers with unsigned certificate. It is a critical problem as 
authentication may involve external servers (for example KMS with self-signed 
certificate). Submitting a workflow in such an environment can result in an 
exception like:

`2018-01-08 10:13:51,471 WARN 
org.apache.hadoop.crypto.key.kms.LoadBalancingKMSClientProvider: 
SERVER[myserver] KMS provider at [https://myserver:16000/kms/v1/] threw an 
IOException: 
javax.net.ssl.SSLHandshakeException: sun.security.validator.ValidatorException: 
PKIX path building failed: 
sun.security.provider.certpath.SunCertPathBuilderException: unable to
 find valid certification path to requested target
at sun.security.ssl.Alerts.getSSLException(Alerts.java:192)
at sun.security.ssl.SSLSocketImpl.fatal(SSLSocketImpl.java:1959)
at sun.security.ssl.Handshaker.fatalSE(Handshaker.java:302)
at sun.security.ssl.Handshaker.fatalSE(Handshaker.java:296)
at 
sun.security.ssl.ClientHandshaker.serverCertificate(ClientHandshaker.java:1514)
at 
sun.security.ssl.ClientHandshaker.processMessage(ClientHandshaker.java:216)
at sun.security.ssl.Handshaker.processLoop(Handshaker.java:1026)
at sun.security.ssl.Handshaker.process_record(Handshaker.java:961)
at sun.security.ssl.SSLSocketImpl.readRecord(SSLSocketImpl.java:1072)
at 
sun.security.ssl.SSLSocketImpl.performInitialHandshake(SSLSocketImpl.java:1385)
at 
sun.security.ssl.SSLSocketImpl.startHandshake(SSLSocketImpl.java:1413)
at 
sun.security.ssl.SSLSocketImpl.startHandshake(SSLSocketImpl.java:1397)
at 
sun.net.www.protocol.https.HttpsClient.afterConnect(HttpsClient.java:559)
at 
sun.net.www.protocol.https.AbstractDelegateHttpsURLConnection.connect(AbstractDelegateHttpsURLConnection.java:185)
at 
sun.net.www.protocol.https.HttpsURLConnectionImpl.connect(HttpsURLConnectionImpl.java:153)
at 
org.apache.hadoop.security.authentication.client.KerberosAuthenticator.authenticate(KerberosAuthenticator.java:186)
at 
org.apache.hadoop.security.token.delegation.web.DelegationTokenAuthenticator.authenticate(DelegationTokenAuthenticator.java:144)
at 
org.apache.hadoop.security.authentication.client.AuthenticatedURL.openConnection(AuthenticatedURL.java:348)
at 
org.apache.hadoop.security.token.delegation.web.DelegationTokenAuthenticatedURL.openConnection(DelegationTokenAuthenticatedURL.java:333)
at 
org.apache.hadoop.crypto.key.kms.KMSClientProvider$1.run(KMSClientProvider.java:477)
at 
org.apache.hadoop.crypto.key.kms.KMSClientProvider$1.run(KMSClientProvider.java:472)
at java.security.AccessController.doPrivileged(Native Method)
at javax.security.auth.Subject.doAs(Subject.java:422)
at 
org.apache.hadoop.security.UserGroupInformation.doAs(UserGroupInformation.java:1962)
at 
org.apache.hadoop.crypto.key.kms.KMSClientProvider.createConnection(KMSClientProvider.java:471)
at 
org.apache.hadoop.crypto.key.kms.KMSClientProvider.decryptEncryptedKey(KMSClientProvider.java:776)
at 
org.apache.hadoop.crypto.key.kms.LoadBalancingKMSClientProvider$5.call(LoadBalancingKMSClientProvider.java:287)
at 
org.apache.hadoop.crypto.key.kms.LoadBalancingKMSClientProvider$5.call(LoadBalancingKMSClientProvider.java:283)
at 
org.apache.hadoop.crypto.key.kms.LoadBalancingKMSClientProvider.doOp(LoadBalancingKMSClientProvider.java:123)
at 
org.apache.hadoop.crypto.key.kms.LoadBalancingKMSClientProvider.decryptEncryptedKey(LoadBalancingKMSClientProvider.java:283)
at 
org.apache.hadoop.crypto.key.KeyProviderCryptoExtension.decryptEncryptedKey(KeyProviderCryptoExtension.java:532)
at 
org.apache.hadoop.hdfs.DFSClient.decryptEncryptedDataEncryptionKey(DFSClient.java:926)
at 
org.apache.hadoop.hdfs.DFSClient.createWrappedInputStream(DFSClient.java:945)
at 
org.apache.hadoop.hdfs.DistributedFileSystem$4.doCall(DistributedFileSystem.java:315)
at 
org.apache.hadoop.hdfs.DistributedFileSystem$4.doCall(DistributedFileSystem.java:310)
at 
org.apache.hadoop.fs.FileSystemLinkResolver.resolve(FileSystemLinkResolver.java:81)
at 
org.apache.hadoop.hdfs.DistributedFileSystem.open(DistributedFileSystem.java:322)
at org.apache.hadoop.fs.FileSystem.open(FileSystem.java:949)
at 

Re: Review Request 65287: OOZIE-3157 - Setup truststore so that it also works in HTTP only mode

2018-01-26 Thread Kinga Marton via Review Board

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

(Updated Jan. 26, 2018, noon)


Review request for oozie, András Piros, Attila Sasvari, and Peter Cseh.


Repository: oozie-git


Description
---

oozie.https.truststore.file is not read and used when oozie.https.enabled is 
false in oozie-site xml. As a result, the Oozie server will be unable to 
communicate with servers with unsigned certificate. It is a critical problem as 
authentication may involve external servers (for example KMS with self-signed 
certificate). Submitting a workflow in such an environment can result in an 
exception like:

`2018-01-08 10:13:51,471 WARN 
org.apache.hadoop.crypto.key.kms.LoadBalancingKMSClientProvider: 
SERVER[myserver] KMS provider at [https://myserver:16000/kms/v1/] threw an 
IOException: 
javax.net.ssl.SSLHandshakeException: sun.security.validator.ValidatorException: 
PKIX path building failed: 
sun.security.provider.certpath.SunCertPathBuilderException: unable to
 find valid certification path to requested target
at sun.security.ssl.Alerts.getSSLException(Alerts.java:192)
at sun.security.ssl.SSLSocketImpl.fatal(SSLSocketImpl.java:1959)
at sun.security.ssl.Handshaker.fatalSE(Handshaker.java:302)
at sun.security.ssl.Handshaker.fatalSE(Handshaker.java:296)
at 
sun.security.ssl.ClientHandshaker.serverCertificate(ClientHandshaker.java:1514)
at 
sun.security.ssl.ClientHandshaker.processMessage(ClientHandshaker.java:216)
at sun.security.ssl.Handshaker.processLoop(Handshaker.java:1026)
at sun.security.ssl.Handshaker.process_record(Handshaker.java:961)
at sun.security.ssl.SSLSocketImpl.readRecord(SSLSocketImpl.java:1072)
at 
sun.security.ssl.SSLSocketImpl.performInitialHandshake(SSLSocketImpl.java:1385)
at 
sun.security.ssl.SSLSocketImpl.startHandshake(SSLSocketImpl.java:1413)
at 
sun.security.ssl.SSLSocketImpl.startHandshake(SSLSocketImpl.java:1397)
at 
sun.net.www.protocol.https.HttpsClient.afterConnect(HttpsClient.java:559)
at 
sun.net.www.protocol.https.AbstractDelegateHttpsURLConnection.connect(AbstractDelegateHttpsURLConnection.java:185)
at 
sun.net.www.protocol.https.HttpsURLConnectionImpl.connect(HttpsURLConnectionImpl.java:153)
at 
org.apache.hadoop.security.authentication.client.KerberosAuthenticator.authenticate(KerberosAuthenticator.java:186)
at 
org.apache.hadoop.security.token.delegation.web.DelegationTokenAuthenticator.authenticate(DelegationTokenAuthenticator.java:144)
at 
org.apache.hadoop.security.authentication.client.AuthenticatedURL.openConnection(AuthenticatedURL.java:348)
at 
org.apache.hadoop.security.token.delegation.web.DelegationTokenAuthenticatedURL.openConnection(DelegationTokenAuthenticatedURL.java:333)
at 
org.apache.hadoop.crypto.key.kms.KMSClientProvider$1.run(KMSClientProvider.java:477)
at 
org.apache.hadoop.crypto.key.kms.KMSClientProvider$1.run(KMSClientProvider.java:472)
at java.security.AccessController.doPrivileged(Native Method)
at javax.security.auth.Subject.doAs(Subject.java:422)
at 
org.apache.hadoop.security.UserGroupInformation.doAs(UserGroupInformation.java:1962)
at 
org.apache.hadoop.crypto.key.kms.KMSClientProvider.createConnection(KMSClientProvider.java:471)
at 
org.apache.hadoop.crypto.key.kms.KMSClientProvider.decryptEncryptedKey(KMSClientProvider.java:776)
at 
org.apache.hadoop.crypto.key.kms.LoadBalancingKMSClientProvider$5.call(LoadBalancingKMSClientProvider.java:287)
at 
org.apache.hadoop.crypto.key.kms.LoadBalancingKMSClientProvider$5.call(LoadBalancingKMSClientProvider.java:283)
at 
org.apache.hadoop.crypto.key.kms.LoadBalancingKMSClientProvider.doOp(LoadBalancingKMSClientProvider.java:123)
at 
org.apache.hadoop.crypto.key.kms.LoadBalancingKMSClientProvider.decryptEncryptedKey(LoadBalancingKMSClientProvider.java:283)
at 
org.apache.hadoop.crypto.key.KeyProviderCryptoExtension.decryptEncryptedKey(KeyProviderCryptoExtension.java:532)
at 
org.apache.hadoop.hdfs.DFSClient.decryptEncryptedDataEncryptionKey(DFSClient.java:926)
at 
org.apache.hadoop.hdfs.DFSClient.createWrappedInputStream(DFSClient.java:945)
at 
org.apache.hadoop.hdfs.DistributedFileSystem$4.doCall(DistributedFileSystem.java:315)
at 
org.apache.hadoop.hdfs.DistributedFileSystem$4.doCall(DistributedFileSystem.java:310)
at 
org.apache.hadoop.fs.FileSystemLinkResolver.resolve(FileSystemLinkResolver.java:81)
at 
org.apache.hadoop.hdfs.DistributedFileSystem.open(DistributedFileSystem.java:322)
at org.apache.hadoop.fs.FileSystem.open(FileSystem.java:949)
at 

Re: Review Request 65287: OOZIE-3157 - Setup truststore so that it also works in HTTP only mode

2018-01-24 Thread Kinga Marton via Review Board


> On Jan. 24, 2018, 1:54 p.m., Attila Sasvari wrote:
> > server/src/main/java/org/apache/oozie/server/EmbeddedOozieServer.java
> > Lines 169 (patched)
> > 
> >
> > What happens if a truststore password starts/ends with whitespaces on 
> > purpose? trim() would remove leading and trailing whitespaces and Oozie 
> > won't be able to open the truststore?

Actually this is how it was in the SSlServerConnectorFactory and I believed 
that that trim was for a purpose there. But as we discussed I removed it, and I 
corrected the keystore part as well from SSlServerConnectorFactory


- Kinga


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


On Jan. 24, 2018, 3:53 p.m., Kinga Marton wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65287/
> ---
> 
> (Updated Jan. 24, 2018, 3:53 p.m.)
> 
> 
> Review request for oozie, András Piros, Attila Sasvari, and Peter Cseh.
> 
> 
> Repository: oozie-git
> 
> 
> Description
> ---
> 
> oozie.https.truststore.file is not read and used when oozie.https.enabled is 
> false in oozie-site xml. As a result, the Oozie server will be unable to 
> communicate with servers with unsigned certificate. It is a critical problem 
> as authentication may involve external servers (for example KMS with 
> self-signed certificate). Submitting a workflow in such an environment can 
> result in an exception like:
> 
> `2018-01-08 10:13:51,471 WARN 
> org.apache.hadoop.crypto.key.kms.LoadBalancingKMSClientProvider: 
> SERVER[myserver] KMS provider at [https://myserver:16000/kms/v1/] threw an 
> IOException: 
> javax.net.ssl.SSLHandshakeException: 
> sun.security.validator.ValidatorException: PKIX path building failed: 
> sun.security.provider.certpath.SunCertPathBuilderException: unable to
>  find valid certification path to requested target
> at sun.security.ssl.Alerts.getSSLException(Alerts.java:192)
> at sun.security.ssl.SSLSocketImpl.fatal(SSLSocketImpl.java:1959)
> at sun.security.ssl.Handshaker.fatalSE(Handshaker.java:302)
> at sun.security.ssl.Handshaker.fatalSE(Handshaker.java:296)
> at 
> sun.security.ssl.ClientHandshaker.serverCertificate(ClientHandshaker.java:1514)
> at 
> sun.security.ssl.ClientHandshaker.processMessage(ClientHandshaker.java:216)
> at sun.security.ssl.Handshaker.processLoop(Handshaker.java:1026)
> at sun.security.ssl.Handshaker.process_record(Handshaker.java:961)
> at sun.security.ssl.SSLSocketImpl.readRecord(SSLSocketImpl.java:1072)
> at 
> sun.security.ssl.SSLSocketImpl.performInitialHandshake(SSLSocketImpl.java:1385)
> at 
> sun.security.ssl.SSLSocketImpl.startHandshake(SSLSocketImpl.java:1413)
> at 
> sun.security.ssl.SSLSocketImpl.startHandshake(SSLSocketImpl.java:1397)
> at 
> sun.net.www.protocol.https.HttpsClient.afterConnect(HttpsClient.java:559)
> at 
> sun.net.www.protocol.https.AbstractDelegateHttpsURLConnection.connect(AbstractDelegateHttpsURLConnection.java:185)
> at 
> sun.net.www.protocol.https.HttpsURLConnectionImpl.connect(HttpsURLConnectionImpl.java:153)
> at 
> org.apache.hadoop.security.authentication.client.KerberosAuthenticator.authenticate(KerberosAuthenticator.java:186)
> at 
> org.apache.hadoop.security.token.delegation.web.DelegationTokenAuthenticator.authenticate(DelegationTokenAuthenticator.java:144)
> at 
> org.apache.hadoop.security.authentication.client.AuthenticatedURL.openConnection(AuthenticatedURL.java:348)
> at 
> org.apache.hadoop.security.token.delegation.web.DelegationTokenAuthenticatedURL.openConnection(DelegationTokenAuthenticatedURL.java:333)
> at 
> org.apache.hadoop.crypto.key.kms.KMSClientProvider$1.run(KMSClientProvider.java:477)
> at 
> org.apache.hadoop.crypto.key.kms.KMSClientProvider$1.run(KMSClientProvider.java:472)
> at java.security.AccessController.doPrivileged(Native Method)
> at javax.security.auth.Subject.doAs(Subject.java:422)
> at 
> org.apache.hadoop.security.UserGroupInformation.doAs(UserGroupInformation.java:1962)
> at 
> org.apache.hadoop.crypto.key.kms.KMSClientProvider.createConnection(KMSClientProvider.java:471)
> at 
> org.apache.hadoop.crypto.key.kms.KMSClientProvider.decryptEncryptedKey(KMSClientProvider.java:776)
> at 
> org.apache.hadoop.crypto.key.kms.LoadBalancingKMSClientProvider$5.call(LoadBalancingKMSClientProvider.java:287)
> at 
> org.apache.hadoop.crypto.key.kms.LoadBalancingKMSClientProvider$5.call(LoadBalancingKMSClientProvider.java:283)
>  

Re: Review Request 65287: OOZIE-3157 - Setup truststore so that it also works in HTTP only mode

2018-01-24 Thread Kinga Marton via Review Board

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

(Updated Jan. 24, 2018, 3:53 p.m.)


Review request for oozie, András Piros, Attila Sasvari, and Peter Cseh.


Repository: oozie-git


Description
---

oozie.https.truststore.file is not read and used when oozie.https.enabled is 
false in oozie-site xml. As a result, the Oozie server will be unable to 
communicate with servers with unsigned certificate. It is a critical problem as 
authentication may involve external servers (for example KMS with self-signed 
certificate). Submitting a workflow in such an environment can result in an 
exception like:

`2018-01-08 10:13:51,471 WARN 
org.apache.hadoop.crypto.key.kms.LoadBalancingKMSClientProvider: 
SERVER[myserver] KMS provider at [https://myserver:16000/kms/v1/] threw an 
IOException: 
javax.net.ssl.SSLHandshakeException: sun.security.validator.ValidatorException: 
PKIX path building failed: 
sun.security.provider.certpath.SunCertPathBuilderException: unable to
 find valid certification path to requested target
at sun.security.ssl.Alerts.getSSLException(Alerts.java:192)
at sun.security.ssl.SSLSocketImpl.fatal(SSLSocketImpl.java:1959)
at sun.security.ssl.Handshaker.fatalSE(Handshaker.java:302)
at sun.security.ssl.Handshaker.fatalSE(Handshaker.java:296)
at 
sun.security.ssl.ClientHandshaker.serverCertificate(ClientHandshaker.java:1514)
at 
sun.security.ssl.ClientHandshaker.processMessage(ClientHandshaker.java:216)
at sun.security.ssl.Handshaker.processLoop(Handshaker.java:1026)
at sun.security.ssl.Handshaker.process_record(Handshaker.java:961)
at sun.security.ssl.SSLSocketImpl.readRecord(SSLSocketImpl.java:1072)
at 
sun.security.ssl.SSLSocketImpl.performInitialHandshake(SSLSocketImpl.java:1385)
at 
sun.security.ssl.SSLSocketImpl.startHandshake(SSLSocketImpl.java:1413)
at 
sun.security.ssl.SSLSocketImpl.startHandshake(SSLSocketImpl.java:1397)
at 
sun.net.www.protocol.https.HttpsClient.afterConnect(HttpsClient.java:559)
at 
sun.net.www.protocol.https.AbstractDelegateHttpsURLConnection.connect(AbstractDelegateHttpsURLConnection.java:185)
at 
sun.net.www.protocol.https.HttpsURLConnectionImpl.connect(HttpsURLConnectionImpl.java:153)
at 
org.apache.hadoop.security.authentication.client.KerberosAuthenticator.authenticate(KerberosAuthenticator.java:186)
at 
org.apache.hadoop.security.token.delegation.web.DelegationTokenAuthenticator.authenticate(DelegationTokenAuthenticator.java:144)
at 
org.apache.hadoop.security.authentication.client.AuthenticatedURL.openConnection(AuthenticatedURL.java:348)
at 
org.apache.hadoop.security.token.delegation.web.DelegationTokenAuthenticatedURL.openConnection(DelegationTokenAuthenticatedURL.java:333)
at 
org.apache.hadoop.crypto.key.kms.KMSClientProvider$1.run(KMSClientProvider.java:477)
at 
org.apache.hadoop.crypto.key.kms.KMSClientProvider$1.run(KMSClientProvider.java:472)
at java.security.AccessController.doPrivileged(Native Method)
at javax.security.auth.Subject.doAs(Subject.java:422)
at 
org.apache.hadoop.security.UserGroupInformation.doAs(UserGroupInformation.java:1962)
at 
org.apache.hadoop.crypto.key.kms.KMSClientProvider.createConnection(KMSClientProvider.java:471)
at 
org.apache.hadoop.crypto.key.kms.KMSClientProvider.decryptEncryptedKey(KMSClientProvider.java:776)
at 
org.apache.hadoop.crypto.key.kms.LoadBalancingKMSClientProvider$5.call(LoadBalancingKMSClientProvider.java:287)
at 
org.apache.hadoop.crypto.key.kms.LoadBalancingKMSClientProvider$5.call(LoadBalancingKMSClientProvider.java:283)
at 
org.apache.hadoop.crypto.key.kms.LoadBalancingKMSClientProvider.doOp(LoadBalancingKMSClientProvider.java:123)
at 
org.apache.hadoop.crypto.key.kms.LoadBalancingKMSClientProvider.decryptEncryptedKey(LoadBalancingKMSClientProvider.java:283)
at 
org.apache.hadoop.crypto.key.KeyProviderCryptoExtension.decryptEncryptedKey(KeyProviderCryptoExtension.java:532)
at 
org.apache.hadoop.hdfs.DFSClient.decryptEncryptedDataEncryptionKey(DFSClient.java:926)
at 
org.apache.hadoop.hdfs.DFSClient.createWrappedInputStream(DFSClient.java:945)
at 
org.apache.hadoop.hdfs.DistributedFileSystem$4.doCall(DistributedFileSystem.java:315)
at 
org.apache.hadoop.hdfs.DistributedFileSystem$4.doCall(DistributedFileSystem.java:310)
at 
org.apache.hadoop.fs.FileSystemLinkResolver.resolve(FileSystemLinkResolver.java:81)
at 
org.apache.hadoop.hdfs.DistributedFileSystem.open(DistributedFileSystem.java:322)
at org.apache.hadoop.fs.FileSystem.open(FileSystem.java:949)
at 

Re: Review Request 65287: OOZIE-3157 - Setup truststore so that it also works in HTTP only mode

2018-01-24 Thread Kinga Marton via Review Board


> On Jan. 23, 2018, 3:17 p.m., Attila Sasvari wrote:
> > Please update documentation and mention truststore related things: it might 
> > be needed even for scenarios without HTTPS (if Oozie needs to talk with 
> > servers with self-signed certificates), config properties can be overriden 
> > via system properties.

Done.


- Kinga


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


On Jan. 24, 2018, 12:19 p.m., Kinga Marton wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65287/
> ---
> 
> (Updated Jan. 24, 2018, 12:19 p.m.)
> 
> 
> Review request for oozie, András Piros, Attila Sasvari, and Peter Cseh.
> 
> 
> Repository: oozie-git
> 
> 
> Description
> ---
> 
> oozie.https.truststore.file is not read and used when oozie.https.enabled is 
> false in oozie-site xml. As a result, the Oozie server will be unable to 
> communicate with servers with unsigned certificate. It is a critical problem 
> as authentication may involve external servers (for example KMS with 
> self-signed certificate). Submitting a workflow in such an environment can 
> result in an exception like:
> 
> `2018-01-08 10:13:51,471 WARN 
> org.apache.hadoop.crypto.key.kms.LoadBalancingKMSClientProvider: 
> SERVER[myserver] KMS provider at [https://myserver:16000/kms/v1/] threw an 
> IOException: 
> javax.net.ssl.SSLHandshakeException: 
> sun.security.validator.ValidatorException: PKIX path building failed: 
> sun.security.provider.certpath.SunCertPathBuilderException: unable to
>  find valid certification path to requested target
> at sun.security.ssl.Alerts.getSSLException(Alerts.java:192)
> at sun.security.ssl.SSLSocketImpl.fatal(SSLSocketImpl.java:1959)
> at sun.security.ssl.Handshaker.fatalSE(Handshaker.java:302)
> at sun.security.ssl.Handshaker.fatalSE(Handshaker.java:296)
> at 
> sun.security.ssl.ClientHandshaker.serverCertificate(ClientHandshaker.java:1514)
> at 
> sun.security.ssl.ClientHandshaker.processMessage(ClientHandshaker.java:216)
> at sun.security.ssl.Handshaker.processLoop(Handshaker.java:1026)
> at sun.security.ssl.Handshaker.process_record(Handshaker.java:961)
> at sun.security.ssl.SSLSocketImpl.readRecord(SSLSocketImpl.java:1072)
> at 
> sun.security.ssl.SSLSocketImpl.performInitialHandshake(SSLSocketImpl.java:1385)
> at 
> sun.security.ssl.SSLSocketImpl.startHandshake(SSLSocketImpl.java:1413)
> at 
> sun.security.ssl.SSLSocketImpl.startHandshake(SSLSocketImpl.java:1397)
> at 
> sun.net.www.protocol.https.HttpsClient.afterConnect(HttpsClient.java:559)
> at 
> sun.net.www.protocol.https.AbstractDelegateHttpsURLConnection.connect(AbstractDelegateHttpsURLConnection.java:185)
> at 
> sun.net.www.protocol.https.HttpsURLConnectionImpl.connect(HttpsURLConnectionImpl.java:153)
> at 
> org.apache.hadoop.security.authentication.client.KerberosAuthenticator.authenticate(KerberosAuthenticator.java:186)
> at 
> org.apache.hadoop.security.token.delegation.web.DelegationTokenAuthenticator.authenticate(DelegationTokenAuthenticator.java:144)
> at 
> org.apache.hadoop.security.authentication.client.AuthenticatedURL.openConnection(AuthenticatedURL.java:348)
> at 
> org.apache.hadoop.security.token.delegation.web.DelegationTokenAuthenticatedURL.openConnection(DelegationTokenAuthenticatedURL.java:333)
> at 
> org.apache.hadoop.crypto.key.kms.KMSClientProvider$1.run(KMSClientProvider.java:477)
> at 
> org.apache.hadoop.crypto.key.kms.KMSClientProvider$1.run(KMSClientProvider.java:472)
> at java.security.AccessController.doPrivileged(Native Method)
> at javax.security.auth.Subject.doAs(Subject.java:422)
> at 
> org.apache.hadoop.security.UserGroupInformation.doAs(UserGroupInformation.java:1962)
> at 
> org.apache.hadoop.crypto.key.kms.KMSClientProvider.createConnection(KMSClientProvider.java:471)
> at 
> org.apache.hadoop.crypto.key.kms.KMSClientProvider.decryptEncryptedKey(KMSClientProvider.java:776)
> at 
> org.apache.hadoop.crypto.key.kms.LoadBalancingKMSClientProvider$5.call(LoadBalancingKMSClientProvider.java:287)
> at 
> org.apache.hadoop.crypto.key.kms.LoadBalancingKMSClientProvider$5.call(LoadBalancingKMSClientProvider.java:283)
> at 
> org.apache.hadoop.crypto.key.kms.LoadBalancingKMSClientProvider.doOp(LoadBalancingKMSClientProvider.java:123)
> at 
> org.apache.hadoop.crypto.key.kms.LoadBalancingKMSClientProvider.decryptEncryptedKey(LoadBalancingKMSClientProvider.java:283)
> at 
> 

Re: Review Request 65287: OOZIE-3157 - Setup truststore so that it also works in HTTP only mode

2018-01-24 Thread Kinga Marton via Review Board

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

(Updated Jan. 24, 2018, 12:19 p.m.)


Review request for oozie, András Piros, Attila Sasvari, and Peter Cseh.


Repository: oozie-git


Description
---

oozie.https.truststore.file is not read and used when oozie.https.enabled is 
false in oozie-site xml. As a result, the Oozie server will be unable to 
communicate with servers with unsigned certificate. It is a critical problem as 
authentication may involve external servers (for example KMS with self-signed 
certificate). Submitting a workflow in such an environment can result in an 
exception like:

`2018-01-08 10:13:51,471 WARN 
org.apache.hadoop.crypto.key.kms.LoadBalancingKMSClientProvider: 
SERVER[myserver] KMS provider at [https://myserver:16000/kms/v1/] threw an 
IOException: 
javax.net.ssl.SSLHandshakeException: sun.security.validator.ValidatorException: 
PKIX path building failed: 
sun.security.provider.certpath.SunCertPathBuilderException: unable to
 find valid certification path to requested target
at sun.security.ssl.Alerts.getSSLException(Alerts.java:192)
at sun.security.ssl.SSLSocketImpl.fatal(SSLSocketImpl.java:1959)
at sun.security.ssl.Handshaker.fatalSE(Handshaker.java:302)
at sun.security.ssl.Handshaker.fatalSE(Handshaker.java:296)
at 
sun.security.ssl.ClientHandshaker.serverCertificate(ClientHandshaker.java:1514)
at 
sun.security.ssl.ClientHandshaker.processMessage(ClientHandshaker.java:216)
at sun.security.ssl.Handshaker.processLoop(Handshaker.java:1026)
at sun.security.ssl.Handshaker.process_record(Handshaker.java:961)
at sun.security.ssl.SSLSocketImpl.readRecord(SSLSocketImpl.java:1072)
at 
sun.security.ssl.SSLSocketImpl.performInitialHandshake(SSLSocketImpl.java:1385)
at 
sun.security.ssl.SSLSocketImpl.startHandshake(SSLSocketImpl.java:1413)
at 
sun.security.ssl.SSLSocketImpl.startHandshake(SSLSocketImpl.java:1397)
at 
sun.net.www.protocol.https.HttpsClient.afterConnect(HttpsClient.java:559)
at 
sun.net.www.protocol.https.AbstractDelegateHttpsURLConnection.connect(AbstractDelegateHttpsURLConnection.java:185)
at 
sun.net.www.protocol.https.HttpsURLConnectionImpl.connect(HttpsURLConnectionImpl.java:153)
at 
org.apache.hadoop.security.authentication.client.KerberosAuthenticator.authenticate(KerberosAuthenticator.java:186)
at 
org.apache.hadoop.security.token.delegation.web.DelegationTokenAuthenticator.authenticate(DelegationTokenAuthenticator.java:144)
at 
org.apache.hadoop.security.authentication.client.AuthenticatedURL.openConnection(AuthenticatedURL.java:348)
at 
org.apache.hadoop.security.token.delegation.web.DelegationTokenAuthenticatedURL.openConnection(DelegationTokenAuthenticatedURL.java:333)
at 
org.apache.hadoop.crypto.key.kms.KMSClientProvider$1.run(KMSClientProvider.java:477)
at 
org.apache.hadoop.crypto.key.kms.KMSClientProvider$1.run(KMSClientProvider.java:472)
at java.security.AccessController.doPrivileged(Native Method)
at javax.security.auth.Subject.doAs(Subject.java:422)
at 
org.apache.hadoop.security.UserGroupInformation.doAs(UserGroupInformation.java:1962)
at 
org.apache.hadoop.crypto.key.kms.KMSClientProvider.createConnection(KMSClientProvider.java:471)
at 
org.apache.hadoop.crypto.key.kms.KMSClientProvider.decryptEncryptedKey(KMSClientProvider.java:776)
at 
org.apache.hadoop.crypto.key.kms.LoadBalancingKMSClientProvider$5.call(LoadBalancingKMSClientProvider.java:287)
at 
org.apache.hadoop.crypto.key.kms.LoadBalancingKMSClientProvider$5.call(LoadBalancingKMSClientProvider.java:283)
at 
org.apache.hadoop.crypto.key.kms.LoadBalancingKMSClientProvider.doOp(LoadBalancingKMSClientProvider.java:123)
at 
org.apache.hadoop.crypto.key.kms.LoadBalancingKMSClientProvider.decryptEncryptedKey(LoadBalancingKMSClientProvider.java:283)
at 
org.apache.hadoop.crypto.key.KeyProviderCryptoExtension.decryptEncryptedKey(KeyProviderCryptoExtension.java:532)
at 
org.apache.hadoop.hdfs.DFSClient.decryptEncryptedDataEncryptionKey(DFSClient.java:926)
at 
org.apache.hadoop.hdfs.DFSClient.createWrappedInputStream(DFSClient.java:945)
at 
org.apache.hadoop.hdfs.DistributedFileSystem$4.doCall(DistributedFileSystem.java:315)
at 
org.apache.hadoop.hdfs.DistributedFileSystem$4.doCall(DistributedFileSystem.java:310)
at 
org.apache.hadoop.fs.FileSystemLinkResolver.resolve(FileSystemLinkResolver.java:81)
at 
org.apache.hadoop.hdfs.DistributedFileSystem.open(DistributedFileSystem.java:322)
at org.apache.hadoop.fs.FileSystem.open(FileSystem.java:949)
at 

Re: Review Request 65287: OOZIE-3157 - Setup truststore so that it also works in HTTP only mode

2018-01-24 Thread Kinga Marton via Review Board

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

(Updated Jan. 24, 2018, 12:17 p.m.)


Review request for oozie, András Piros, Attila Sasvari, and Peter Cseh.


Repository: oozie-git


Description
---

oozie.https.truststore.file is not read and used when oozie.https.enabled is 
false in oozie-site xml. As a result, the Oozie server will be unable to 
communicate with servers with unsigned certificate. It is a critical problem as 
authentication may involve external servers (for example KMS with self-signed 
certificate). Submitting a workflow in such an environment can result in an 
exception like:

`2018-01-08 10:13:51,471 WARN 
org.apache.hadoop.crypto.key.kms.LoadBalancingKMSClientProvider: 
SERVER[myserver] KMS provider at [https://myserver:16000/kms/v1/] threw an 
IOException: 
javax.net.ssl.SSLHandshakeException: sun.security.validator.ValidatorException: 
PKIX path building failed: 
sun.security.provider.certpath.SunCertPathBuilderException: unable to
 find valid certification path to requested target
at sun.security.ssl.Alerts.getSSLException(Alerts.java:192)
at sun.security.ssl.SSLSocketImpl.fatal(SSLSocketImpl.java:1959)
at sun.security.ssl.Handshaker.fatalSE(Handshaker.java:302)
at sun.security.ssl.Handshaker.fatalSE(Handshaker.java:296)
at 
sun.security.ssl.ClientHandshaker.serverCertificate(ClientHandshaker.java:1514)
at 
sun.security.ssl.ClientHandshaker.processMessage(ClientHandshaker.java:216)
at sun.security.ssl.Handshaker.processLoop(Handshaker.java:1026)
at sun.security.ssl.Handshaker.process_record(Handshaker.java:961)
at sun.security.ssl.SSLSocketImpl.readRecord(SSLSocketImpl.java:1072)
at 
sun.security.ssl.SSLSocketImpl.performInitialHandshake(SSLSocketImpl.java:1385)
at 
sun.security.ssl.SSLSocketImpl.startHandshake(SSLSocketImpl.java:1413)
at 
sun.security.ssl.SSLSocketImpl.startHandshake(SSLSocketImpl.java:1397)
at 
sun.net.www.protocol.https.HttpsClient.afterConnect(HttpsClient.java:559)
at 
sun.net.www.protocol.https.AbstractDelegateHttpsURLConnection.connect(AbstractDelegateHttpsURLConnection.java:185)
at 
sun.net.www.protocol.https.HttpsURLConnectionImpl.connect(HttpsURLConnectionImpl.java:153)
at 
org.apache.hadoop.security.authentication.client.KerberosAuthenticator.authenticate(KerberosAuthenticator.java:186)
at 
org.apache.hadoop.security.token.delegation.web.DelegationTokenAuthenticator.authenticate(DelegationTokenAuthenticator.java:144)
at 
org.apache.hadoop.security.authentication.client.AuthenticatedURL.openConnection(AuthenticatedURL.java:348)
at 
org.apache.hadoop.security.token.delegation.web.DelegationTokenAuthenticatedURL.openConnection(DelegationTokenAuthenticatedURL.java:333)
at 
org.apache.hadoop.crypto.key.kms.KMSClientProvider$1.run(KMSClientProvider.java:477)
at 
org.apache.hadoop.crypto.key.kms.KMSClientProvider$1.run(KMSClientProvider.java:472)
at java.security.AccessController.doPrivileged(Native Method)
at javax.security.auth.Subject.doAs(Subject.java:422)
at 
org.apache.hadoop.security.UserGroupInformation.doAs(UserGroupInformation.java:1962)
at 
org.apache.hadoop.crypto.key.kms.KMSClientProvider.createConnection(KMSClientProvider.java:471)
at 
org.apache.hadoop.crypto.key.kms.KMSClientProvider.decryptEncryptedKey(KMSClientProvider.java:776)
at 
org.apache.hadoop.crypto.key.kms.LoadBalancingKMSClientProvider$5.call(LoadBalancingKMSClientProvider.java:287)
at 
org.apache.hadoop.crypto.key.kms.LoadBalancingKMSClientProvider$5.call(LoadBalancingKMSClientProvider.java:283)
at 
org.apache.hadoop.crypto.key.kms.LoadBalancingKMSClientProvider.doOp(LoadBalancingKMSClientProvider.java:123)
at 
org.apache.hadoop.crypto.key.kms.LoadBalancingKMSClientProvider.decryptEncryptedKey(LoadBalancingKMSClientProvider.java:283)
at 
org.apache.hadoop.crypto.key.KeyProviderCryptoExtension.decryptEncryptedKey(KeyProviderCryptoExtension.java:532)
at 
org.apache.hadoop.hdfs.DFSClient.decryptEncryptedDataEncryptionKey(DFSClient.java:926)
at 
org.apache.hadoop.hdfs.DFSClient.createWrappedInputStream(DFSClient.java:945)
at 
org.apache.hadoop.hdfs.DistributedFileSystem$4.doCall(DistributedFileSystem.java:315)
at 
org.apache.hadoop.hdfs.DistributedFileSystem$4.doCall(DistributedFileSystem.java:310)
at 
org.apache.hadoop.fs.FileSystemLinkResolver.resolve(FileSystemLinkResolver.java:81)
at 
org.apache.hadoop.hdfs.DistributedFileSystem.open(DistributedFileSystem.java:322)
at org.apache.hadoop.fs.FileSystem.open(FileSystem.java:949)
at 

Review Request 65287: OOZIE-3157 - Setup truststore so that it also works in HTTP only mode

2018-01-23 Thread Kinga Marton via Review Board

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

Review request for oozie, András Piros, Attila Sasvari, and Peter Cseh.


Repository: oozie-git


Description
---

oozie.https.truststore.file is not read and used when oozie.https.enabled is 
false in oozie-site xml. As a result, the Oozie server will be unable to 
communicate with servers with unsigned certificate. It is a critical problem as 
authentication may involve external servers (for example KMS with self-signed 
certificate). Submitting a workflow in such an environment can result in an 
exception like:

`2018-01-08 10:13:51,471 WARN 
org.apache.hadoop.crypto.key.kms.LoadBalancingKMSClientProvider: 
SERVER[myserver] KMS provider at [https://myserver:16000/kms/v1/] threw an 
IOException: 
javax.net.ssl.SSLHandshakeException: sun.security.validator.ValidatorException: 
PKIX path building failed: 
sun.security.provider.certpath.SunCertPathBuilderException: unable to
 find valid certification path to requested target
at sun.security.ssl.Alerts.getSSLException(Alerts.java:192)
at sun.security.ssl.SSLSocketImpl.fatal(SSLSocketImpl.java:1959)
at sun.security.ssl.Handshaker.fatalSE(Handshaker.java:302)
at sun.security.ssl.Handshaker.fatalSE(Handshaker.java:296)
at 
sun.security.ssl.ClientHandshaker.serverCertificate(ClientHandshaker.java:1514)
at 
sun.security.ssl.ClientHandshaker.processMessage(ClientHandshaker.java:216)
at sun.security.ssl.Handshaker.processLoop(Handshaker.java:1026)
at sun.security.ssl.Handshaker.process_record(Handshaker.java:961)
at sun.security.ssl.SSLSocketImpl.readRecord(SSLSocketImpl.java:1072)
at 
sun.security.ssl.SSLSocketImpl.performInitialHandshake(SSLSocketImpl.java:1385)
at 
sun.security.ssl.SSLSocketImpl.startHandshake(SSLSocketImpl.java:1413)
at 
sun.security.ssl.SSLSocketImpl.startHandshake(SSLSocketImpl.java:1397)
at 
sun.net.www.protocol.https.HttpsClient.afterConnect(HttpsClient.java:559)
at 
sun.net.www.protocol.https.AbstractDelegateHttpsURLConnection.connect(AbstractDelegateHttpsURLConnection.java:185)
at 
sun.net.www.protocol.https.HttpsURLConnectionImpl.connect(HttpsURLConnectionImpl.java:153)
at 
org.apache.hadoop.security.authentication.client.KerberosAuthenticator.authenticate(KerberosAuthenticator.java:186)
at 
org.apache.hadoop.security.token.delegation.web.DelegationTokenAuthenticator.authenticate(DelegationTokenAuthenticator.java:144)
at 
org.apache.hadoop.security.authentication.client.AuthenticatedURL.openConnection(AuthenticatedURL.java:348)
at 
org.apache.hadoop.security.token.delegation.web.DelegationTokenAuthenticatedURL.openConnection(DelegationTokenAuthenticatedURL.java:333)
at 
org.apache.hadoop.crypto.key.kms.KMSClientProvider$1.run(KMSClientProvider.java:477)
at 
org.apache.hadoop.crypto.key.kms.KMSClientProvider$1.run(KMSClientProvider.java:472)
at java.security.AccessController.doPrivileged(Native Method)
at javax.security.auth.Subject.doAs(Subject.java:422)
at 
org.apache.hadoop.security.UserGroupInformation.doAs(UserGroupInformation.java:1962)
at 
org.apache.hadoop.crypto.key.kms.KMSClientProvider.createConnection(KMSClientProvider.java:471)
at 
org.apache.hadoop.crypto.key.kms.KMSClientProvider.decryptEncryptedKey(KMSClientProvider.java:776)
at 
org.apache.hadoop.crypto.key.kms.LoadBalancingKMSClientProvider$5.call(LoadBalancingKMSClientProvider.java:287)
at 
org.apache.hadoop.crypto.key.kms.LoadBalancingKMSClientProvider$5.call(LoadBalancingKMSClientProvider.java:283)
at 
org.apache.hadoop.crypto.key.kms.LoadBalancingKMSClientProvider.doOp(LoadBalancingKMSClientProvider.java:123)
at 
org.apache.hadoop.crypto.key.kms.LoadBalancingKMSClientProvider.decryptEncryptedKey(LoadBalancingKMSClientProvider.java:283)
at 
org.apache.hadoop.crypto.key.KeyProviderCryptoExtension.decryptEncryptedKey(KeyProviderCryptoExtension.java:532)
at 
org.apache.hadoop.hdfs.DFSClient.decryptEncryptedDataEncryptionKey(DFSClient.java:926)
at 
org.apache.hadoop.hdfs.DFSClient.createWrappedInputStream(DFSClient.java:945)
at 
org.apache.hadoop.hdfs.DistributedFileSystem$4.doCall(DistributedFileSystem.java:315)
at 
org.apache.hadoop.hdfs.DistributedFileSystem$4.doCall(DistributedFileSystem.java:310)
at 
org.apache.hadoop.fs.FileSystemLinkResolver.resolve(FileSystemLinkResolver.java:81)
at 
org.apache.hadoop.hdfs.DistributedFileSystem.open(DistributedFileSystem.java:322)
at org.apache.hadoop.fs.FileSystem.open(FileSystem.java:949)
at 
org.apache.oozie.service.AuthorizationService.authorizeForApp(AuthorizationService.java:392)
at 

Re: Review Request 65177: >git status< should be clean after >mvn test< was called

2018-01-18 Thread Kinga Marton via Review Board

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

(Updated Jan. 18, 2018, 11:35 a.m.)


Review request for oozie, András Piros, Attila Sasvari, and Peter Cseh.


Repository: oozie-git


Description
---

After running TestDistcpMain, local files created are not removed. Subsequent 
test runs will reuse those files and fail:
`$ ls -lrt /home/asasvari/workspace/oozie/core/distcp*`
`-rw-rw-r-- 1 asasvari asasvari 1025 Dec 21 18:25 
/home/asasvari/workspace/oozie/core/distcp-log4j.properties`
`-rw-rw-r-- 1 asasvari asasvari 5756 Dec 21 18:26 
/home/asasvari/workspace/oozie/core/distcp-oozie-1513876931701.log`
Those files should be removed after test execution. It would be better to use 
the oozie/core/target directory so that mvn could clean it.


Diffs (updated)
-

  core/src/test/java/org/apache/oozie/action/hadoop/TestLauncher.java a3b3b7b3 
  core/src/test/java/org/apache/oozie/client/TestOozieCLI.java 8c2aa982 
  core/src/test/java/org/apache/oozie/jms/TestJMSJobEventListener.java 728916ec 
  core/src/test/java/org/apache/oozie/service/TestJMSAccessorService.java 
505049d2 
  core/src/test/java/org/apache/oozie/test/XTestCase.java 8cecb89b 
  sharelib/hive/src/test/java/org/apache/oozie/action/hadoop/TestHiveMain.java 
35c2865e 
  sharelib/pig/src/test/java/org/apache/oozie/action/hadoop/TestPigMain.java 
9a185c91 
  
sharelib/spark/src/test/java/org/apache/oozie/action/hadoop/TestJarFilter.java 
ff1b3cef 
  
sharelib/spark/src/test/java/org/apache/oozie/action/hadoop/TestSparkMain.java 
b9f37c84 
  tools/src/test/java/org/apache/oozie/tools/TestOozieMySqlDBCLI.java a7df5c79 


Diff: https://reviews.apache.org/r/65177/diff/2/

Changes: https://reviews.apache.org/r/65177/diff/1-2/


Testing
---

I`ve run a mvn clean test and the repository remained clean.


Thanks,

Kinga Marton



Review Request 65177: >git status< should be clean after >mvn test< was called

2018-01-16 Thread Kinga Marton via Review Board

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

Review request for oozie, András Piros, Attila Sasvari, and Peter Cseh.


Repository: oozie-git


Description
---

After running TestDistcpMain, local files created are not removed. Subsequent 
test runs will reuse those files and fail:
`$ ls -lrt /home/asasvari/workspace/oozie/core/distcp*`
`-rw-rw-r-- 1 asasvari asasvari 1025 Dec 21 18:25 
/home/asasvari/workspace/oozie/core/distcp-log4j.properties`
`-rw-rw-r-- 1 asasvari asasvari 5756 Dec 21 18:26 
/home/asasvari/workspace/oozie/core/distcp-oozie-1513876931701.log`
Those files should be removed after test execution. It would be better to use 
the oozie/core/target directory so that mvn could clean it.


Diffs
-

  core/src/test/java/org/apache/oozie/action/hadoop/TestLauncher.java a3b3b7b3 
  core/src/test/java/org/apache/oozie/client/TestOozieCLI.java 8c2aa982 
  core/src/test/java/org/apache/oozie/jms/TestJMSJobEventListener.java 728916ec 
  core/src/test/java/org/apache/oozie/service/TestJMSAccessorService.java 
505049d2 
  core/src/test/java/org/apache/oozie/test/XTestCase.java 8cecb89b 
  sharelib/hive/src/test/java/org/apache/oozie/action/hadoop/TestHiveMain.java 
35c2865e 
  sharelib/pig/src/test/java/org/apache/oozie/action/hadoop/TestPigMain.java 
9a185c91 
  
sharelib/spark/src/test/java/org/apache/oozie/action/hadoop/TestJarFilter.java 
ff1b3cef 
  
sharelib/spark/src/test/java/org/apache/oozie/action/hadoop/TestSparkMain.java 
b9f37c84 
  tools/src/test/java/org/apache/oozie/tools/TestOozieMySqlDBCLI.java a7df5c79 


Diff: https://reviews.apache.org/r/65177/diff/1/


Testing
---

I`ve run a mvn clean test and the repository remained clean.


Thanks,

Kinga Marton



Re: Review Request 64808: OOZIE-3085 - Improve logging in ActionExecutors

2018-01-11 Thread Kinga Marton via Review Board

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

(Updated Jan. 11, 2018, 4:02 p.m.)


Review request for oozie and Attila Sasvari.


Repository: oozie-git


Description
---

Decision, ForkJoin, Email and FS ActionExecutors are producing no or very 
little logs on info level. This makes it hard to gather usage information and 
help with troubleshooting in the case of any issues. Please improve logging in 
these classes.
Several subtasks can be separated out from this one


Diffs (updated)
-

  
core/src/main/java/org/apache/oozie/action/control/ControlNodeActionExecutor.java
 cc1b6108 
  
core/src/main/java/org/apache/oozie/action/decision/DecisionActionExecutor.java 
8c235bda 
  core/src/main/java/org/apache/oozie/action/email/EmailActionExecutor.java 
d59f1d76 
  core/src/main/java/org/apache/oozie/action/hadoop/FsActionExecutor.java 
63f6104d 
  core/src/main/java/org/apache/oozie/action/hadoop/JavaActionExecutor.java 
5c8acae5 
  
core/src/main/java/org/apache/oozie/action/oozie/SubWorkflowActionExecutor.java 
d62cf686 
  core/src/main/java/org/apache/oozie/action/ssh/SshActionExecutor.java 
5890b8c1 


Diff: https://reviews.apache.org/r/64808/diff/3/

Changes: https://reviews.apache.org/r/64808/diff/2-3/


Testing
---

Manually tested the log entries for each affected ActionExecutor.
The log messages look like: 

`2018-01-11 16:24:06,967  INFO ForkActionExecutor:520 - 
SERVER[kmarton-MBP.local] USER[martonjuliakinga] GROUP[-] TOKEN[] 
APP[Fork_workflow] JOB[001-18055450212-oozie-mart-W] 
ACTION[001-18055450212-oozie-mart-W@fs-paralel] Starting action
`

`2018-01-11 14:40:22,770  INFO FsActionExecutor:520 - SERVER[kmarton-MBP.local] 
USER[martonjuliakinga] GROUP[-] TOKEN[] APP[one-op-wf] 
JOB[006-18034310829-oozie-mart-W] 
ACTION[006-18034310829-oozie-mart-W@action1] Action ended with external 
status [OK]
`


Thanks,

Kinga Marton



Re: Review Request 64808: OOZIE-3085 - Improve logging in ActionExecutors

2018-01-11 Thread Kinga Marton via Review Board


> On Dec. 28, 2017, 11:44 a.m., Attila Sasvari wrote:
> > core/src/main/java/org/apache/oozie/action/control/ControlNodeActionExecutor.java
> > Lines 52 (patched)
> > 
> >
> > What does ``getId()`` return for a control node action?

the getId() for the ActionExecutors as I saw are following the pattern: 
JobId@ActionName. In case of the ControlNodeActionExecutors looks like: 
000-171228181538280-oozie-mart-W@end, 
000-171228181538280-oozie-mart-W@:start:, 
004-171228181538280-oozie-mart-W@fs-paralel (this was my fork action), 
004-171228181538280-oozie-mart-W@join-fs (this was my join action). But as 
we agreed I will remove it from the log message because is duplicated.


> On Dec. 28, 2017, 11:44 a.m., Attila Sasvari wrote:
> > core/src/main/java/org/apache/oozie/action/control/ControlNodeActionExecutor.java
> > Lines 59 (patched)
> > 
> >
> > What does ``getExternalStatus()`` return for a control node action? It 
> > would might sense to log ``getStatus()`` too.

In case of ControlNodeACtionExecutor the status is the same as the 
externalStatus (OK). It will be redundant to write both of them. There are 
cases when the two values are different, but the status is determined based on 
the value of the external status


> On Dec. 28, 2017, 11:44 a.m., Attila Sasvari wrote:
> > core/src/main/java/org/apache/oozie/action/decision/DecisionActionExecutor.java
> > Line 45 (original), 47 (patched)
> > 
> >
> > What does ``getId()`` return for a control node action? I've just 
> > checked TestDecisionActionExecutor (executed) and saw it was ``null`` that 
> > does not seem very useful. 
> > 
> > Can you create a workflow with all changed control nodes and try it out 
> > on a cluster (pseudo hadoop should be fine) and double check logs about 
> > starting and ending actions?

the getId() for the ActionExecutors as I saw are following the pattern: 
JobId@ActionName, where the action name comes from the workflow.xml, and here 
the name is mandatory. But as we discussed I will remove it from the log 
message because is duplicated.


> On Dec. 28, 2017, 11:44 a.m., Attila Sasvari wrote:
> > core/src/main/java/org/apache/oozie/action/decision/DecisionActionExecutor.java
> > Lines 83 (patched)
> > 
> >
> > There is a difference between status ``action.getStatus()`` (that can 
> > be "OK", "KILLED", "FAILED", etc.) and ``getExternalStatus()`` (in case of 
> > a decision action it returns where the execution will flow, transition to 
> > the next workflow action). 
> > 
> > For example:
> > ```
> > 
> >  false
> >  false
> >  false
> >  
> >  > ```
> > 
> > would return ``OK`` for ``action.getStatus()`` and ``d``for 
> > ``action.getExternalStatus()``. It might help to log both.

Yes, you are right. The external status will return the transition. As we 
discussed it will be unnecessary to log both ot them.


> On Dec. 28, 2017, 11:44 a.m., Attila Sasvari wrote:
> > core/src/main/java/org/apache/oozie/action/email/EmailActionExecutor.java
> > Lines 107 (patched)
> > 
> >
> > What does ``getId()`` return?

JobId@ActionName, where the action name comes from the workflow.xml. But as we 
discussed I will remove it from the log message because is duplicated.


> On Dec. 28, 2017, 11:44 a.m., Attila Sasvari wrote:
> > core/src/main/java/org/apache/oozie/action/email/EmailActionExecutor.java
> > Lines 313 (patched)
> > 
> >
> > What does ``getExternalStatus()`` return? It would might sense to log 
> > ``getStatus()`` too.

In this case the two values are the same


> On Dec. 28, 2017, 11:44 a.m., Attila Sasvari wrote:
> > core/src/main/java/org/apache/oozie/action/hadoop/FsActionExecutor.java
> > Lines 630 (patched)
> > 
> >
> > What does ``getId()`` return?

JobId@ActionName, where the action name comes from the workflow.xml. But as we 
discussed I will remove it from the log message because is duplicated.


> On Dec. 28, 2017, 11:44 a.m., Attila Sasvari wrote:
> > core/src/main/java/org/apache/oozie/action/oozie/SubWorkflowActionExecutor.java
> > Lines 167 (patched)
> > 
> >
> > What does ``getId()`` return?

JobId@ActionName, where the action name comes from the workflow.xml. But as we 
discussed I will remove it from the log message because is duplicated.


> On Dec. 28, 2017, 11:44 a.m., Attila Sasvari wrote:
> > 

Re: Review Request 64808: OOZIE-3085 - Improve logging in ActionExecutors

2018-01-11 Thread Kinga Marton via Review Board

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

(Updated Jan. 11, 2018, 4:02 p.m.)


Review request for oozie and Attila Sasvari.


Repository: oozie-git


Description
---

Decision, ForkJoin, Email and FS ActionExecutors are producing no or very 
little logs on info level. This makes it hard to gather usage information and 
help with troubleshooting in the case of any issues. Please improve logging in 
these classes.
Several subtasks can be separated out from this one


Diffs
-

  
core/src/main/java/org/apache/oozie/action/control/ControlNodeActionExecutor.java
 cc1b6108 
  
core/src/main/java/org/apache/oozie/action/decision/DecisionActionExecutor.java 
8c235bda 
  core/src/main/java/org/apache/oozie/action/email/EmailActionExecutor.java 
d59f1d76 
  core/src/main/java/org/apache/oozie/action/hadoop/FsActionExecutor.java 
63f6104d 
  
core/src/main/java/org/apache/oozie/action/oozie/SubWorkflowActionExecutor.java 
d62cf686 
  core/src/main/java/org/apache/oozie/action/ssh/SshActionExecutor.java 
5890b8c1 


Diff: https://reviews.apache.org/r/64808/diff/2/


Testing (updated)
---

Manually tested the log entries for each affected ActionExecutor.
The log messages look like: 

`2018-01-11 16:24:06,967  INFO ForkActionExecutor:520 - 
SERVER[kmarton-MBP.local] USER[martonjuliakinga] GROUP[-] TOKEN[] 
APP[Fork_workflow] JOB[001-18055450212-oozie-mart-W] 
ACTION[001-18055450212-oozie-mart-W@fs-paralel] Starting action
`

`2018-01-11 14:40:22,770  INFO FsActionExecutor:520 - SERVER[kmarton-MBP.local] 
USER[martonjuliakinga] GROUP[-] TOKEN[] APP[one-op-wf] 
JOB[006-18034310829-oozie-mart-W] 
ACTION[006-18034310829-oozie-mart-W@action1] Action ended with external 
status [OK]
`


Thanks,

Kinga Marton



  1   2   >