SameerMesiah97 commented on code in PR #71652:
URL: https://github.com/apache/airflow/pull/71652#discussion_r3793049863
##########
providers/amazon/src/airflow/providers/amazon/aws/sensors/emr.py:
##########
@@ -123,7 +124,11 @@ class
EmrServerlessJobSensor(AwsBaseSensor[EmrServerlessHook]):
:param application_id: application_id to check the state of
:param job_run_id: job_run_id to check the state of
- :param target_states: a set of states to wait for, defaults to 'SUCCESS'
+ :param target_states: a set of states to wait for, defaults to
``SUCCESS``. In deferrable mode
+ the waiter waits for a terminal success state regardless of
``target_states``.
Review Comment:
Why are the termination semantics of deferrable mode not symmetrical to
non-deferrable? As implemented, a sensor configured with e.g.
`target_states={"RUNNING"}` can succeed at `RUNNING` in synchronous mode but
wait until job completion in deferrable mode. Can we preserve `target_states`
semantics in the trigger?
##########
providers/amazon/src/airflow/providers/amazon/aws/sensors/emr.py:
##########
@@ -123,7 +124,11 @@ class
EmrServerlessJobSensor(AwsBaseSensor[EmrServerlessHook]):
:param application_id: application_id to check the state of
:param job_run_id: job_run_id to check the state of
- :param target_states: a set of states to wait for, defaults to 'SUCCESS'
+ :param target_states: a set of states to wait for, defaults to
``SUCCESS``. In deferrable mode
+ the waiter waits for a terminal success state regardless of
``target_states``.
+ :param max_attempts: Maximum waiter attempts in deferrable mode. Ignored
when
+ ``deferrable=False``; poke mode uses ``timeout`` (default
``sensors.default_timeout``, 7 days).
+ :param deferrable: Run sensor in the deferrable mode.
Review Comment:
I am not sure about `max_attempts` only applying when `deferrable=True`.
This gives the sensor different timeout semantics depending on execution mode:
non-deferrable execution is bounded by the sensor timeout, while deferrable
execution is bounded by `max_attempts` . Could we use the same timeout
semantics for both modes rather than introducing a deferrable-specific limit?
--
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]
