molcay commented on PR #41329: URL: https://github.com/apache/airflow/pull/41329#issuecomment-2518434562
> > This PR will add the ideal signal sequence for task runner termination mentioned here: [#35474 (comment)](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. I read the discussion and thank you for the discussion, it was illuminating for me :) As far as I understand, there is no need for this implementation (after #44465), right? -- 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]
