o-nikolas commented on code in PR #40377:
URL: https://github.com/apache/airflow/pull/40377#discussion_r1674554754
##########
tests/providers/ssh/hooks/test_ssh.py:
##########
@@ -1068,7 +1068,12 @@ def
test_ssh_connection_with_no_host_key_check_true_and_allow_host_key_changes_f
ssh_mock.reset_mock()
with mock.patch("os.path.isfile", return_value=False):
+ # Reset ssh hook to initial state
+ hook = SSHHook(
+
ssh_conn_id=self.CONN_SSH_WITH_NO_HOST_KEY_CHECK_TRUE_AND_ALLOW_HOST_KEY_CHANGES_FALSE
+ )
with hook.get_conn():
+ print("TESTS", ssh_mock.return_value)
Review Comment:
Debug statement left in the code
##########
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:
I'm confused why you're adding this to execute. `on_kill` should be called
automatically in cases where the task is killed by signal or timeout. Why are
we also calling it here? Was the execute method always leaving connections open
even when it was completing gracefully?
--
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]
