[GitHub] [airflow] argibbs commented on a diff in pull request #29933: Explicit skipped states list for ExternalTaskSensor

["argibbs (via GitHub)" <gi...@apache.org>]

argibbs commented on code in PR #29933:
URL: https://github.com/apache/airflow/pull/29933#discussion_r1126159457


##########
airflow/sensors/external_task.py:
##########
@@ -273,6 +284,31 @@ def poke(self, context, session=None):
                     )
                 raise AirflowException(f"The external DAG {self.external_dag_id} failed.")
 
+        count_skipped = -1
+        if self.skipped_states:
+            count_skipped = self.get_count(dttm_filter, session, self.skipped_states)
+
+        # Skip if anything in the list has skipped. Note if we are checking multiple tasks and one skips
+        # before another errors, we'll skip first.

Review Comment:
   This initial change is simply fixing the problem I have where I only have a single monitored task (or dag). As you're noting, Multiple tasks complicates things - I did give this some thought hence the comment in the code...
   
   > Maybe we can improve the sensor by adding something similar to TriggerRule
   Can you expand with an explicit example? Given that this change is entirely additive in terms of behaviour, I'm concerned about scope creep. Or to put it another way, YAGNI. This PR solves a problem I actually have. I don't want to go and complicate the change without an actual use case, which realistically isn't going to come from me. If this makes it into the main branch, and people start using it and then ask for more levers on the behaviour (e.g. something like a min-skip-count threshold), I'd want to make the change then.
   
   > or just skip it when nothing is fail and we have at least one task in skipped state.
   I believe this is what the code does? If anything's failed, we'll raise an AirflowException and fail. If not, we then check for skips, and if anything's skipped we'll raise an AirflowSkipException. (And then obviously we check if everything's in a good state, and go good if so)



-- 
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: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org