kaxil commented on code in PR #61627:
URL: https://github.com/apache/airflow/pull/61627#discussion_r3324916181


##########
task-sdk/src/airflow/sdk/execution_time/supervisor.py:
##########
@@ -2232,7 +2232,34 @@ def supervise_task(
                 sentry_integration=sentry_integration,
             )
 
-            exit_code = process.wait()
+            # Forward termination signals to the task subprocess so that the 
operator's
+            # on_kill() hook is invoked on graceful shutdown (e.g. K8s pod 
SIGTERM).
+            # Without this, the supervisor exits on SIGTERM without notifying 
the child,
+            # leaving spawned resources (pods, subprocesses, etc.) running.
+            prev_sigterm = signal.getsignal(signal.SIGTERM)

Review Comment:
   This still isn't applied at HEAD (`0880eb03`). Lines 1398-1399 keep the two 
`signal.getsignal()` lookups and then lines 1410-1411 call `signal.signal()` 
separately. Since `signal.signal()` returns the previous handler, move the 
`_forward_signal` definition above the install and drop both `getsignal()` 
lines:
   
   ```python
   def _forward_signal(signum, frame):
       ...
   
   prev_sigterm = signal.signal(signal.SIGTERM, _forward_signal)
   prev_sigint = signal.signal(signal.SIGINT, _forward_signal)
   ```
   
   Minor, not blocking -- just flagging since it was marked done.



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