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




core/src/main/java/org/apache/oozie/workflow/lite/LiteWorkflowValidator.java 
(line 158)
<https://reviews.apache.org/r/50202/#comment208599>

    Note: I know that this method has too many arguments.
    
    It doesn't look nice. A possible approach is to move some of the stuff 
(maps, app) to class variables. However I don't want to store state in this 
class - we only need these stuff during validation. Of course we could go ahead 
and re-create the maps & app variables during each validateWorkflow() call but 
I don't like that either.
    
    If someone has a good idea, don't hold it back.



core/src/main/java/org/apache/oozie/workflow/lite/LiteWorkflowValidator.java 
(line 294)
<https://reviews.apache.org/r/50202/#comment208600>

    I think I can get rid of this. Once the loop is detected, just add the node 
to the end of the deque and call the Joiner in-place.



core/src/test/java/org/apache/oozie/workflow/lite/TestLiteWorkflowAppParser.java
 (line 1380)
<https://reviews.apache.org/r/50202/#comment208601>

    I'm no longer using E0732. This assumes that it's possible to decide which 
is the proper Join node for a given fork. I don't think it's possible to 
correctly decide this in the current implementation. So I dropped this and 
introduced two new codes.


- Peter Bacsko


On júl. 19, 2016, 7:56 du, Peter Bacsko wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50202/
> -----------------------------------------------------------
> 
> (Updated júl. 19, 2016, 7:56 du)
> 
> 
> Review request for oozie.
> 
> 
> Bugs: OOZIE-1978
>     https://issues.apache.org/jira/browse/OOZIE-1978
> 
> 
> Repository: oozie-git
> 
> 
> Description
> -------
> 
> See OOZIE-1978 for details.
> 
> 
> Diffs
> -----
> 
>   core/src/main/java/org/apache/oozie/ErrorCode.java 2907ca2 
>   
> core/src/main/java/org/apache/oozie/workflow/lite/LiteWorkflowAppParser.java 
> a1b9cdb 
>   
> core/src/main/java/org/apache/oozie/workflow/lite/LiteWorkflowValidator.java 
> PRE-CREATION 
>   core/src/test/java/org/apache/oozie/command/wf/TestSubmitXCommand.java 
> 73464c8 
>   
> core/src/test/java/org/apache/oozie/workflow/lite/TestLiteWorkflowAppParser.java
>  9002b6c 
> 
> Diff: https://reviews.apache.org/r/50202/diff/
> 
> 
> Testing
> -------
> 
> Existing tests pass.
> 
> We might need to add some more, at least testing acyclic graph detection.
> 
> 
> Thanks,
> 
> Peter Bacsko
> 
>

Reply via email to