yuqian90 edited a comment on issue #7098: [AIRFLOW-4453] Make behavior of 
`none_failed` consistent with documentation
URL: https://github.com/apache/airflow/pull/7098#issuecomment-581009288
 
 
   @TV4Fun this sounds like a good suggestion. How about calling the new 
trigger rule `all_success_or_skip` ? That makes it easier to comprehend.
   
   > 1. Keep the current behavior of `none_failed` and create a new trigger 
rule (`none_failed_including_skips` maybe? I don't know) that behaves as in 
this PR. I saw keep the current behavior for `none_failed` so as to minimize 
the amount of refactoring we make people do, even though from a documentation 
perspective, it would make more sense to change the behavior of `none_failed` 
and move the current behavior to a new operator like `none_failed_one_success` 
or something like that just because this behavior is more in line with the 
intuitive reading of `none_failed` and less likely to cause confusion.
   
    I do suggest we update the documentation about `none_failed` trigger rule 
to match what it actually does. Current the doc says this:
   ```
   none_failed: all parents have not failed (failed or upstream_failed) i.e. 
all parents have succeeded or been skipped
   ```
   To describe `none_failed` more precisely, it should say something like this 
instead:
   ```
   none_failed: all parents have not failed (failed or upstream_failed) i.e. 
all parents have succeeded or been skipped. However if all parents are skipped, 
the task with none_failed trigger_rule will also be skipped.
   ```
   
   The doc also has a paragraph that says this, which is not 100% accurate:
   ```
   The join task will be triggered as soon as branch_false has been skipped (a 
valid completion state) and follow_branch_a has succeeded. Because skipped 
tasks will not cascade through none_failed.
   ```
   A more accurate description is like:
   ```
   The join task will be triggered as soon as branch_false has been skipped (a 
valid completion state) and follow_branch_a has succeeded. Because skipped 
tasks will not cascade through none_failed as long as some other parent tasks 
have succeeded.
   ```
   
   Whereas the new trigger_rule you are going to add can described something 
like this:
   ```
   all_success_or_skip: All parents have succeeded or been skipped. If all 
parents are skipped, the task with `all_success_or_skip` still succeeds.
   ```
   
   Since this involves updating the documentation and no change to the behavior 
of the existing `none_failed` trigger_rule, it should minimize the amount of 
changes users have to make.
   
   The addition of `all_success_or_skip` can be made in the same PR or a 
different one. It will make it possible to achieve what this PR originally want 
to do, having a trigger_rule that succeeds whenever parents are successful or 
skipped.
   

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
[email protected]


With regards,
Apache Git Services

Reply via email to