shahar1 commented on code in PR #41561:
URL: https://github.com/apache/airflow/pull/41561#discussion_r1721020824
##########
airflow/providers/amazon/aws/operators/emr.py:
##########
@@ -833,11 +833,9 @@ def execute(self, context: Context) -> str | None:
waiter_max_attempts=self.waiter_max_attempts,
),
method_name="execute_complete",
- # timeout is set to ensure that if a trigger dies, the timeout
does not restart
- # 60 seconds is added to allow the trigger to exit gracefully
(i.e. yield TriggerEvent)
Review Comment:
Why did you delete these comments?
##########
airflow/providers/amazon/aws/operators/emr.py:
##########
@@ -824,7 +824,7 @@ def execute(self, context: Context) -> str | None:
job_flow_id=self._job_flow_id,
log_uri=get_log_uri(emr_client=self._emr_hook.conn,
job_flow_id=self._job_flow_id),
)
- if self.deferrable:
+ if self.deferrable and self.wait_for_completion:
Review Comment:
1. I'd consider refactoring both conditions as follows (a bit more readable
IMO):
```python
if self.wait_for_completion:
if self.deferrable:
...
else:
...
```
2. Unit tests + docstring update for this behavior would be helpful.
--
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]
