seanmuth commented on PR #65738:
URL: https://github.com/apache/airflow/pull/65738#issuecomment-4847519004

   Following up on @ashb’s review note about the session-leader TODO:
   
   Confirmed this PR does make the task-runner a session leader — `os.setsid()` 
is added right after the fork in `start()`, and `kill()` now signals the whole 
group via `os.killpg(os.getpgid(self._process.pid), sig)` with a 
`send_signal()` fallback (plus a test asserting the child’s PGID == its own PID 
post-fork).
   
   That makes the pre-existing `# TODO: Make this process a session leader` in 
`_fork_main` obsolete, but the PR doesn’t currently touch it:
   
https://github.com/apache/airflow/blob/0bfc7eac0e8506795d06b73d65943ccc0f4e82d5/task-sdk/src/airflow/sdk/execution_time/supervisor.py#L420
   
   Could you drop that TODO comment as part of this change? Thanks!


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