hussein-awala commented on code in PR #33718:
URL: https://github.com/apache/airflow/pull/33718#discussion_r1306150639
##########
airflow/sensors/base.py:
##########
@@ -285,6 +286,8 @@ def run_duration() -> float:
def resume_execution(self, next_method: str, next_kwargs: dict[str, Any] |
None, context: Context):
try:
return super().resume_execution(next_method, next_kwargs, context)
+ except TaskDeferralTimeout as e:
+ raise AirflowSensorTimeout(*e.args) from e
Review Comment:
Sensor is an operator, so it accept execution_timeout as param, and
currently in sync mode, when we provide:
retries=2
execution_timeout=1h
retry_delay=30m
timeout=3h30
The sensor will runs for 1h, timeout, retries after 30m, runs for 1h,
timeout, retries after 30m and timeout after 30m with failed state because the
total time is 3h30.
> deferral timeout is used for sensor timeout and it just means one thing --
it means there is no other reason but sensor timeout.
In your case you fail sensor when one of the execution timeout or the
timeout is reached. Is allowing providing execution_time in base_sensor is a
bug for you?
If not, we will have a difference between handling these two configs between
sync and async modes, and that is what I mean by inconsistent.
--
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]
