ashb commented on PR #41329:
URL: https://github.com/apache/airflow/pull/41329#issuecomment-2506098454

   > This PR will add the ideal signal sequence for task runner termination 
mentioned here: 
https://github.com/apache/airflow/issues/35474#issuecomment-1801988073.
   
   @potiuk Why do we want to send a SIGHUP first? That seems like a very odd 
signal to send. SIGINT followed by SIGTERM is what Docker/podman/every other 
container runtime(?) does and feels like a much more normal sequence of 
signals. Especially given that SIGINT being sent to a python process will raise 
a KeyboardInterupt signal won't it, where as by default SIGHUP would just kill 
the process outright (as it doesn't have a default signal handler, so default 
behaviour is to just exit!)
   
   @molcay The new implementation has now been merged and lives in 
https://github.com/apache/airflow/blob/main/task_sdk/src/airflow/sdk/execution_time/supervisor.py
 (as of today only LocalExecutor has been ported over to use this new code 
path, other executors will follow soon.) However before spending time to update 
this PR I'd like Jarek to answer the above Q.


-- 
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]

Reply via email to