You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by GitBox <gi...@apache.org> on 2021/04/21 06:48:11 UTC

[GitHub] [airflow] r-richmond opened a new pull request #15467: Mark tasks as failed when using ONE_SUCCESS and Upstream fails to have one success

r-richmond opened a new pull request #15467:
URL: https://github.com/apache/airflow/pull/15467


   # What I think this does
   1. When tasks use trigger_rule = `ONE_SUCCESS` Mark them as upstream_failed when no upstreams succeed
       * Leaves skipped behavior when all upstream are skipped
   1. Updates several tests I found that I believe test this
   # Why do this
   1. I had several dags that weren't failing out as expected. 
   1. Other users appear to have run into this same issue https://stackoverflow.com/questions/64603004/aiflow-skipping-task-on-one-success-trigger-rule 
   


-- 
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:
users@infra.apache.org



[GitHub] [airflow] ashb merged pull request #15467: Mark tasks as failed when using ONE_SUCCESS and Upstream has no successes

Posted by GitBox <gi...@apache.org>.
ashb merged pull request #15467:
URL: https://github.com/apache/airflow/pull/15467


   


-- 
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:
users@infra.apache.org



[GitHub] [airflow] ashb commented on a change in pull request #15467: Mark tasks as failed when using ONE_SUCCESS and Upstream has no successes

Posted by GitBox <gi...@apache.org>.
ashb commented on a change in pull request #15467:
URL: https://github.com/apache/airflow/pull/15467#discussion_r623718532



##########
File path: airflow/ti_deps/deps/trigger_rule_dep.py
##########
@@ -134,8 +134,12 @@ def _evaluate_trigger_rule(  # pylint: disable=too-many-branches
                 if successes or skipped:
                     ti.set_state(State.SKIPPED, session)
             elif trigger_rule == TR.ONE_SUCCESS:
-                if upstream_done and not successes:
+                if upstream_done and done == skipped:
+                    # if upstream is done and all are skipped mark as skipped
                     ti.set_state(State.SKIPPED, session)
+                elif upstream_done and successes <= 0:

Review comment:
       How can successes be less than zero?




-- 
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:
users@infra.apache.org



[GitHub] [airflow] kaxil closed pull request #15467: Mark tasks as failed when using ONE_SUCCESS and Upstream fails to have one success

Posted by GitBox <gi...@apache.org>.
kaxil closed pull request #15467:
URL: https://github.com/apache/airflow/pull/15467


   


-- 
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:
users@infra.apache.org



[GitHub] [airflow] r-richmond commented on a change in pull request #15467: Mark tasks as failed when using ONE_SUCCESS and Upstream has no successes

Posted by GitBox <gi...@apache.org>.
r-richmond commented on a change in pull request #15467:
URL: https://github.com/apache/airflow/pull/15467#discussion_r623927859



##########
File path: airflow/ti_deps/deps/trigger_rule_dep.py
##########
@@ -134,8 +134,12 @@ def _evaluate_trigger_rule(  # pylint: disable=too-many-branches
                 if successes or skipped:
                     ti.set_state(State.SKIPPED, session)
             elif trigger_rule == TR.ONE_SUCCESS:
-                if upstream_done and not successes:
+                if upstream_done and done == skipped:
+                    # if upstream is done and all are skipped mark as skipped
                     ti.set_state(State.SKIPPED, session)
+                elif upstream_done and successes <= 0:

Review comment:
       Unclear to me; I'm just following the pattern I see below so that this is consistent.
   
   https://github.com/apache/airflow/blob/5e79b1ed7551345c8b2ffebbef6c873e98058a14/airflow/ti_deps/deps/trigger_rule_dep.py#L154-L156




-- 
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:
users@infra.apache.org



[GitHub] [airflow] kaxil commented on pull request #15467: Mark tasks as failed when using ONE_SUCCESS and Upstream fails to have one success

Posted by GitBox <gi...@apache.org>.
kaxil commented on pull request #15467:
URL: https://github.com/apache/airflow/pull/15467#issuecomment-824320072


   Closed/reopened to trigger CI job again


-- 
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:
users@infra.apache.org



[GitHub] [airflow] ashb commented on a change in pull request #15467: Mark tasks as failed when using ONE_SUCCESS and Upstream has no successes

Posted by GitBox <gi...@apache.org>.
ashb commented on a change in pull request #15467:
URL: https://github.com/apache/airflow/pull/15467#discussion_r625614142



##########
File path: airflow/ti_deps/deps/trigger_rule_dep.py
##########
@@ -134,8 +134,12 @@ def _evaluate_trigger_rule(  # pylint: disable=too-many-branches
                 if successes or skipped:
                     ti.set_state(State.SKIPPED, session)
             elif trigger_rule == TR.ONE_SUCCESS:
-                if upstream_done and not successes:
+                if upstream_done and done == skipped:
+                    # if upstream is done and all are skipped mark as skipped
                     ti.set_state(State.SKIPPED, session)
+                elif upstream_done and successes <= 0:

Review comment:
       Fair




-- 
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:
users@infra.apache.org



[GitHub] [airflow] r-richmond closed pull request #15467: Mark tasks as failed when using ONE_SUCCESS and Upstream has no successes

Posted by GitBox <gi...@apache.org>.
r-richmond closed pull request #15467:
URL: https://github.com/apache/airflow/pull/15467


   


-- 
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:
users@infra.apache.org



[GitHub] [airflow] r-richmond commented on a change in pull request #15467: Mark tasks as failed when using ONE_SUCCESS and Upstream has no successes

Posted by GitBox <gi...@apache.org>.
r-richmond commented on a change in pull request #15467:
URL: https://github.com/apache/airflow/pull/15467#discussion_r623927859



##########
File path: airflow/ti_deps/deps/trigger_rule_dep.py
##########
@@ -134,8 +134,12 @@ def _evaluate_trigger_rule(  # pylint: disable=too-many-branches
                 if successes or skipped:
                     ti.set_state(State.SKIPPED, session)
             elif trigger_rule == TR.ONE_SUCCESS:
-                if upstream_done and not successes:
+                if upstream_done and done == skipped:
+                    # if upstream is done and all are skipped mark as skipped
                     ti.set_state(State.SKIPPED, session)
+                elif upstream_done and successes <= 0:

Review comment:
       Unclear to me; I'm just following the pattern I see below so that this is consistent.
   
   https://github.com/apache/airflow/blob/5e79b1ed7551345c8b2ffebbef6c873e98058a14/airflow/ti_deps/deps/trigger_rule_dep.py#L155




-- 
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:
users@infra.apache.org