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




core/src/main/java/org/apache/oozie/action/control/ControlNodeActionExecutor.java
Lines 52 (patched)
<https://reviews.apache.org/r/64808/#comment273404>

    What does ``getId()`` return for a control node action?



core/src/main/java/org/apache/oozie/action/control/ControlNodeActionExecutor.java
Lines 59 (patched)
<https://reviews.apache.org/r/64808/#comment273405>

    What does ``getExternalStatus()`` return for a control node action? It 
would might sense to log ``getStatus()`` too.



core/src/main/java/org/apache/oozie/action/decision/DecisionActionExecutor.java
Line 45 (original), 47 (patched)
<https://reviews.apache.org/r/64808/#comment273402>

    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?



core/src/main/java/org/apache/oozie/action/decision/DecisionActionExecutor.java
Lines 83 (patched)
<https://reviews.apache.org/r/64808/#comment273403>

    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:
    ```
    <switch xmlns='uri:oozie:workflow:0.1'>
     <case to='a'>false</case>
     <case to='b'>false</case>
     <case to='c'>false</case>
     <default to='d'/>
    </switch
    ```
    
    would return ``OK`` for ``action.getStatus()`` and ``d``for 
``action.getExternalStatus()``. It might help to log both.



core/src/main/java/org/apache/oozie/action/email/EmailActionExecutor.java
Lines 107 (patched)
<https://reviews.apache.org/r/64808/#comment273409>

    What does ``getId()`` return?



core/src/main/java/org/apache/oozie/action/email/EmailActionExecutor.java
Lines 313 (patched)
<https://reviews.apache.org/r/64808/#comment273406>

    What does ``getExternalStatus()`` return? It would might sense to log 
``getStatus()`` too.



core/src/main/java/org/apache/oozie/action/hadoop/FsActionExecutor.java
Lines 630 (patched)
<https://reviews.apache.org/r/64808/#comment273410>

    What does ``getId()`` return?



core/src/main/java/org/apache/oozie/action/oozie/SubWorkflowActionExecutor.java
Lines 167 (patched)
<https://reviews.apache.org/r/64808/#comment273411>

    What does ``getId()`` return?



core/src/main/java/org/apache/oozie/action/oozie/SubWorkflowActionExecutor.java
Lines 259 (patched)
<https://reviews.apache.org/r/64808/#comment273408>

    What does ``getExternalStatus()`` return? It would might sense to log 
``getStatus()`` too.



core/src/main/java/org/apache/oozie/action/ssh/SshActionExecutor.java
Lines 467 (patched)
<https://reviews.apache.org/r/64808/#comment273407>

    What does ``getExternalStatus()`` return? It would might sense to log 
``getStatus()`` too.


- Attila Sasvari


On Dec. 27, 2017, 9:12 a.m., Kinga Marton wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/64808/
> -----------------------------------------------------------
> 
> (Updated Dec. 27, 2017, 9:12 a.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
> -------
> 
> 
> Thanks,
> 
> Kinga Marton
> 
>

Reply via email to