dabla commented on code in PR #76528:
URL: https://github.com/apache/airflow/pull/76528#discussion_r4517192035
##########
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.
Review Comment:
You're right: asyncssh 2.18 moved `none` first, and on 2.12 to 2.17 the
default already put `[email protected]` first. The comment now says the explicit
list offers zlib first on every asyncssh version, and the PR description says
what changes on each version range. 79b0400b4e
##########
providers/ssh/docs/connections/ssh.rst:
##########
@@ -51,6 +51,8 @@ Extra (optional)
* ``timeout`` - Deprecated - use conn_timeout instead.
* ``cmd_timeout`` - Timeout (in seconds) for executing the command. The
default is 10 seconds. `null` value means no timeout.
* ``compress`` - ``true`` to ask the remote client/server to compress
traffic; ``false`` to refuse compression. Default is ``true``.
+ The async hook ignores ``Compression`` from ``~/.ssh/config``, and each
compressed connection holds about 0.3 MB of zlib state;
Review Comment:
Right, `SSHHook` takes only `ProxyCommand` and `IdentityFile` from
`~/.ssh/config`. The doc now says neither hook reads `Compression` from it.
79b0400b4e
##########
providers/ssh/docs/connections/ssh.rst:
##########
@@ -51,6 +51,8 @@ Extra (optional)
* ``timeout`` - Deprecated - use conn_timeout instead.
* ``cmd_timeout`` - Timeout (in seconds) for executing the command. The
default is 10 seconds. `null` value means no timeout.
* ``compress`` - ``true`` to ask the remote client/server to compress
traffic; ``false`` to refuse compression. Default is ``true``.
+ The async hook ignores ``Compression`` from ``~/.ssh/config``, and each
compressed connection holds about 0.3 MB of zlib state;
+ set ``false`` on triggerers running many SSH remote job triggers.
Review Comment:
Agreed, it's per connection. The doc now says to set `false` on the
connection the `SSHRemoteJobOperator` jobs use, and that this also turns
compression off for every other task on that connection. 79b0400b4e
--
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]
