potiuk commented on code in PR #40377:
URL: https://github.com/apache/airflow/pull/40377#discussion_r1675900982


##########
airflow/providers/ssh/operators/ssh.py:
##########
@@ -192,9 +192,20 @@ def execute(self, context=None) -> bytes | str:
         enable_pickling = conf.getboolean("core", "enable_xcom_pickling")
         if not enable_pickling:
             result = b64encode(result).decode("utf-8")
+
+        self.on_kill()

Review Comment:
   Yes. We definitnely do not need to call on_kill here. SSHClient is a 
"Closing context manager", so in normal circumstances .close() should be called 
when leaving `with` clause. But when signal (TERM) is received it **might** 
happen (I think, I am not 100% sure actually) that the `__exit__` method is not 
going to be called for the context manager. I tried to get some definitive 
answer on the latter, but in any case, explicitly calling close during signal 
handling in on_kill will not hurt.



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