Lee-W commented on code in PR #40924:
URL: https://github.com/apache/airflow/pull/40924#discussion_r1687305708
##########
airflow/sensors/base.py:
##########
@@ -114,15 +118,33 @@ def _orig_start_date(
)
+class SkipPolicy(str, enum.Enum):
Review Comment:
I feel we might need to update documentation for this as well
##########
airflow/sensors/base.py:
##########
@@ -186,11 +208,44 @@ def __init__(
self.mode = mode
self.exponential_backoff = exponential_backoff
self.max_wait = self._coerce_max_wait(max_wait)
- if soft_fail is True and never_fail is True:
- raise ValueError("soft_fail and never_fail are mutually exclusive,
you can not provide both.")
+ if skip_policy != NOTSET:
+ if sum([soft_fail, silent_fail]) > 0:
+ raise ValueError(
+ "skip_policy and deprecated soft_fail and silent_fail
parameters are mutually exclusive."
+ )
+
+ if skip_policy == SkipPolicy.SKIP_ON_SOFT_ERROR:
+ self.soft_fail = True
Review Comment:
If we're already using `skip_policy` to check. Do we still need this
assignment here?
##########
airflow/sensors/base.py:
##########
@@ -114,15 +118,33 @@ def _orig_start_date(
)
+class SkipPolicy(str, enum.Enum):
+ """Class with sensor's skip policies."""
+
+ # if poke method raise an exception, sensor will not be skipped on.
+ NONE = "none"
+
+ # If poke method raises an exception, sensor will be skipped on.
+ SKIP_ON_ANY_ERROR = "skip_on_any_error"
+
+ # If poke method raises AirflowSensorTimeout, AirflowTaskTimeout,
AirflowFailException
+ # sensor will be skipped on.
+ SKIP_ON_SOFT_ERROR = "skip_on_soft_error"
+
+ # If poke method raises an exception different from AirflowSensorTimeout,
AirflowTaskTimeout,
+ # AirflowSkipException, sensor will ignore exception and re-poke until
timeout.
+ IGNORE_ERROR = "ignore_error"
Review Comment:
I'm wondering whether we could reword this. It looks similar to
`SKIP_ON_ANY_ERROR` at the first glance 🤔
##########
airflow/sensors/base.py:
##########
@@ -114,15 +118,33 @@ def _orig_start_date(
)
+class SkipPolicy(str, enum.Enum):
+ """Class with sensor's skip policies."""
+
+ # if poke method raise an exception, sensor will not be skipped on.
+ NONE = "none"
+
+ # If poke method raises an exception, sensor will be skipped on.
+ SKIP_ON_ANY_ERROR = "skip_on_any_error"
+
+ # If poke method raises AirflowSensorTimeout, AirflowTaskTimeout,
AirflowFailException
+ # sensor will be skipped on.
+ SKIP_ON_SOFT_ERROR = "skip_on_soft_error"
Review Comment:
I might suggest rewording this one as well. The description here is great,
but `soft`_fail sounds a bit unclear to me
--
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]
