ephraimbuddy commented on code in PR #26518:
URL: https://github.com/apache/airflow/pull/26518#discussion_r976841708
##########
airflow/ti_deps/deps/trigger_rule_dep.py:
##########
@@ -162,6 +166,9 @@ def _evaluate_trigger_rule(
changed = ti.set_state(State.UPSTREAM_FAILED, session)
elif skipped:
changed = ti.set_state(State.SKIPPED, session)
+ elif removed and successes and ti.map_index > -1:
+ if ti.map_index >= successes:
Review Comment:
> Hmm, yes now you mention it this feels like it's going to break in some
other cases. Like what if there is 1 mapped upstream which is in the failed
state, one in the removed state, this would erroneously remove it I think?
In this case, `successes` will be `0`, also failed=`1`, so the condition
will not be reached and the taskinstance will be marked as `upstream_failed`.
Same thing when we have skipped task instances. The condition to mark the task
instance as removed will not be reached.
The condition for the task to be marked successful is if we have some
`removed` task instances and `successful` task instances, `no failed`, `no
skipped` and the task is mapped. So if we get here, if the `map_index` of the
task instance is >= all successful task instances, it means the task instance
upstream is removed because indexes go from -1 upwards, it's not possible to
remove map_index 1 and still have map_index 3?
If we have 5 mapped tasks(0,1,2,3,4), and we remove 2, we will have 3 mapped
tasks(0,1,2). If these 3 are successful,(successes=3), then the removed are
those greater than or equal to the map index 3(3,4).
--
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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
