Re: [PR] Add deferrable_timeout to inntialize _timeout_sec of TaskStateTrigger [airflow]

["Xiroo (via GitHub)" <gi...@apache.org>]

Xiroo commented on PR #34715:
URL: https://github.com/apache/airflow/pull/34715#issuecomment-2007580451

   @hussein-awala 
   
   From the behavior you mentioned, it seems that whether I use execution_timeout or timeout to set _timeout_sec, they both lead to the same termination conditions. Upon further reflection, I'm questioning the need for explicitly checking if the dag_run is in a running state using _timeout_sec. This check appears to produce an outcome that overlaps with what is already achieved through the existing timeout parameter of ExeternalTaskSensor.
   
   I want to change the async def run method internal loop logic like following .
   FROM
   ```python
   while True:
       delta = utcnow() - self.trigger_start_time
       if delta.total_seconds() < self._timeout_sec:
           # mypy confuses typing here
           if await self.count_running_dags() == 0:  # type: ignore[call-arg]
               self.log.info("Waiting for DAG to start execution...")
               await asyncio.sleep(self.poll_interval)
       else:
           yield TriggerEvent({"status": "timeout"})
           return
       # mypy confuses typing here
       if await self.count_tasks() == len(self.execution_dates):  # type: ignore[call-arg]
           yield TriggerEvent({"status": "success"})
           return
       self.log.info("Task is still running, sleeping for %s seconds...", self.poll_interval)
       await asyncio.sleep(self.poll_interval)
   ``` 
   TO
   
   ```python
   while True:
       if await self.count_running_dags() == 0:  # type: ignore[call-arg]
           self.log.info("Waiting for DAG to start execution...")
           await asyncio.sleep(self.poll_interval)
       # mypy confuses typing here
       if await self.count_tasks() == len(self.execution_dates):  # type: ignore[call-arg]
           yield TriggerEvent({"status": "success"})
           return
       self.log.info("Task is still running, sleeping for %s seconds...", self.poll_interval)
       await asyncio.sleep(self.poll_interval)
   ``` 
   Instead of making immediate changes, I try adding a deprecation message in this PR. I believe this approach will allow for a smoother transition and provide users with adequate notice before the functionality is eventually removed.
   
   If there's anything I've misunderstood, please let me know.


-- 
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: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org