dabla commented on code in PR #76528:
URL: https://github.com/apache/airflow/pull/76528#discussion_r4478874133
##########
providers/ssh/tests/unit/ssh/hooks/test_ssh_async.py:
##########
@@ -197,6 +198,58 @@ async def test_get_conn_builds_config(self):
assert call_kwargs["username"] == "testuser"
assert result == mock_ssh_client
+ @mock.patch("asyncssh.connect", new_callable=mock.AsyncMock)
+ @mock.patch(
+ "airflow.providers.ssh.hooks.ssh.get_async_connection",
+ new_callable=mock.AsyncMock,
+ )
+ @pytest.mark.asyncio
+ async def test_get_conn_reads_extra_asynchronously(self,
mock_get_connection, mock_connect):
+ """``extra_dejson`` masks synchronously, so the async hook reads the
extra through common.compat."""
+ mock_get_connection.return_value = _connection({"no_host_key_check":
"true"})
+ hook = SSHHookAsync(ssh_conn_id="test_conn")
+
+ with (
+ mock.patch(
+ "airflow.providers.ssh.hooks.ssh.get_async_extra_dejson",
+ new_callable=mock.AsyncMock,
+ return_value={"no_host_key_check": "true"},
+ ) as mock_get_extra,
+ mock.patch.object(
+ Connection, "extra_dejson", new_callable=mock.PropertyMock,
side_effect=AssertionError
+ ) as mock_extra_dejson,
+ ):
+ await hook._get_conn()
+
+
mock_get_extra.assert_awaited_once_with(mock_get_connection.return_value)
+ mock_extra_dejson.assert_not_called()
+ assert mock_connect.call_args.kwargs["known_hosts"] is None
Review Comment:
Done in b9412398cb: the connection is now built with `_connection({})`, so
the test only passes when the asynchronously read extra reaches `_parse_extras`
(it fails with `_get_conn` parsing `json.loads(conn.extra)`). The patches are
decorators, with `autospec=True` on `get_async_connection` and
`get_async_extra_dejson`, here and in
`test_get_conn_compresses_like_sync_hook`. `asyncssh.connect` stays
`new_callable=mock.AsyncMock`: it is wrapped by `@async_context_manager`, so
`autospec` would give a mock that cannot be awaited. A comment above the patch
says so.
##########
providers/ssh/tests/unit/ssh/hooks/test_ssh_async.py:
##########
@@ -66,7 +71,7 @@ def test_parse_extras_key_file(self):
mock_conn.extra_dejson = {"key_file": "/path/to/key"}
mock_conn.host = "test.host"
- hook._parse_extras(mock_conn)
+ hook._parse_extras(mock_conn, mock_conn.extra_dejson)
Review Comment:
Done in b9412398cb: all six now call the helper on a real `Connection`, so
every `_parse_extras` test has the same setup and the bare `MagicMock`s are
gone.
##########
providers/ssh/tests/unit/ssh/hooks/test_ssh_async.py:
##########
@@ -32,6 +32,11 @@ def _connection(extra: dict, host: str = "test.host") ->
Connection:
return Connection(conn_id="test_conn", conn_type="ssh", host=host,
extra=json.dumps(extra))
+def _parse_extras(hook: SSHHookAsync, extra: dict, host: str = "test.host") ->
None:
Review Comment:
Renamed to `_parse_extra_dict` in b9412398cb.
##########
providers/ssh/src/airflow/providers/ssh/hooks/ssh.py:
##########
@@ -658,6 +664,8 @@ def _parse_extras(self, conn: Any) -> None:
)
if "private_key" in extra_options:
self.private_key = extra_options["private_key"]
+ if str(extra_options.get("compress", "true")).lower() == "false":
Review Comment:
Applied in b9412398cb: same expression as `SSHHook` now.
##########
providers/ssh/src/airflow/providers/ssh/hooks/ssh.py:
##########
@@ -722,6 +732,8 @@ def _get_value(self_val, conn_val, default=None):
"port": _get_value(self.port, conn.port, SSH_PORT),
"username": _get_value(self.username, conn.login),
"password": _get_value(self.password, conn.password),
+ # asyncssh prefers no compression; offer zlib first as paramiko
does for SSHHook.
+ "compression_algs": ["[email protected]", "zlib", "none"] if
self.compress else ["none"],
Review Comment:
Added both points under `compress` in `docs/connections/ssh.rst` in
b9412398cb, and a line on the `~/.ssh/config` behaviour change in the PR
description.
--
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]
