shahar1 commented on code in PR #40924:
URL: https://github.com/apache/airflow/pull/40924#discussion_r1686819545
##########
airflow/sensors/base.py:
##########
@@ -114,15 +118,27 @@ def _orig_start_date(
)
+class SkipPolicy(str, enum.Enum):
+ """Action to TODO."""
Review Comment:
Comment is a unclear
##########
airflow/sensors/base.py:
##########
@@ -155,8 +171,8 @@ class BaseSensorOperator(BaseOperator, SkipMixin):
and AirflowFailException, the sensor will log the error and continue
its execution. Otherwise, the sensor task fails, and it can be retried
based on the provided `retries` parameter.
- :param never_fail: If true, and poke method raises an exception, sensor
will be skipped.
- Mutually exclusive with soft_fail.
+ Mutually exclusive with soft_fail.
+ :param skip_policy:
Review Comment:
Description needs to be added, probably with details about each option in
the enum.
I'd include answers to the following questions:
1. What is a "soft" error?
2. What's the difference between "ignore" and "skip"? (it might not be as
trivial for the user)
##########
airflow/sensors/base.py:
##########
@@ -114,15 +118,27 @@ def _orig_start_date(
)
+class SkipPolicy(str, enum.Enum):
+ """Action to TODO."""
+
+ NONE = "none"
+
+ # If poke method raises an exception, sensor will be skipped.
+ SKIP_ON_ANY_ERROR = "skip_on_any_error"
+
+ SKIP_ONLY_SOFT_ERROR = "skip_only_soft_error"
Review Comment:
Namings might need some minor improvements:
```suggestion
SKIP_ON_SOFT_ERROR = "skip_on_soft_error"
```
##########
airflow/sensors/base.py:
##########
@@ -114,15 +118,27 @@ def _orig_start_date(
)
+class SkipPolicy(str, enum.Enum):
+ """Action to TODO."""
+
+ NONE = "none"
+
+ # If poke method raises an exception, sensor will be skipped.
+ SKIP_ON_ANY_ERROR = "skip_on_any_error"
+
+ SKIP_ONLY_SOFT_ERROR = "skip_only_soft_error"
+ IGNORE_ERRORS = "ignore_error"
+
+
class BaseSensorOperator(BaseOperator, SkipMixin):
"""
Sensor operators are derived from this class and inherit these attributes.
Sensor operators keep executing at a time interval and succeed when
a criteria is met and fail if and when they time out.
- :param soft_fail: Set to true to mark the task as SKIPPED on failure.
- Mutually exclusive with never_fail.
+ :param soft_fail: Set to true to mark the task as SKIPPED on
AirflowSensorTimeout,
+ AirflowTaskTimeout, AirflowFailException.
Review Comment:
1. Mutual exclusiveness with `silent_fail` should be clarified.
2. It should be marked as deprecated.
--
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]
