Taragolis commented on code in PR #29347:
URL: https://github.com/apache/airflow/pull/29347#discussion_r1111243238
##########
airflow/providers/ssh/hooks/ssh.py:
##########
@@ -173,6 +182,14 @@ def __init__(
if "conn_timeout" in extra_options and self.conn_timeout is
None:
self.conn_timeout = int(extra_options["conn_timeout"])
+ if self.cmd_timeout is None:
+ if "cmd_timeout" in extra_options:
+ self.cmd_timeout = (
+ int(extra_options["cmd_timeout"]) if
extra_options["cmd_timeout"] else None
+ )
+ else:
+ self.cmd_timeout = CMD_TIMEOUT
+
Review Comment:
> To pass "no timeout" via hook parameters you should pass None as a hook
argument.
To pass "no timeout" via connection parameters you should specify null in
connection extra: {"cmd_timeout": null}. (It's possible to use empty string or
negative values instead of null here, but I feel like it'll be even more
inconsistent with hook parameters).
I can't see any inconsistent here if you want so set no timeout, there is no
problem to set it to `None` or json `null` explicitly, rather than use negative
values which converted to `None` anyway.
NOTSET use as sentinel in situation when None value itself it is a legit
value but for some reason we can't provide `None` as default value, and in this
case it is 100% legit because [paramico use `None` as no
timeout](https://docs.paramiko.org/en/stable/api/client.html#paramiko.client.SSHClient.exec_command).
Potentially this discussion about `spaces vs tabs`
--
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]
