You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by GitBox <gi...@apache.org> on 2022/10/21 12:29:50 UTC

[GitHub] [airflow] punx120 opened a new issue, #27182: SSHOperator ignores cmd_timeout

punx120 opened a new issue, #27182:
URL: https://github.com/apache/airflow/issues/27182

   ### Apache Airflow Provider(s)
   
   ssh
   
   ### Versions of Apache Airflow Providers
   
   _No response_
   
   ### Apache Airflow version
   
   2.4.1
   
   ### Operating System
   
   linux
   
   ### Deployment
   
   Other
   
   ### Deployment details
   
   _No response_
   
   ### What happened
   
   Hi,
   
   SSHOperator documentation states that we should be using cmd_timeout instead of timeout
   ```
   :param timeout: (deprecated) timeout (in seconds) for executing the command. The default is 10 seconds.
           Use conn_timeout and cmd_timeout parameters instead.
   ```
   
   But the code doesn't use cmd_timeout at all - and it's still passing `self.timeout` when running the ssh command:
   ```
   return self.ssh_hook.exec_ssh_client_command(
               ssh_client, command, timeout=self.timeout, environment=self.environment, get_pty=self.get_pty
           )
   ```
   
   It seems to me that we should `self.cmd_timeout` here instead. When creating the hook, it correctly uses `self.conn_timeout`.
   
   I'll try to work on a PR for this.
   
   
   ### What you think should happen instead
   
   _No response_
   
   ### How to reproduce
   
   _No response_
   
   ### Anything else
   
   _No response_
   
   ### Are you willing to submit PR?
   
   - [X] Yes I am willing to submit a PR!
   
   ### Code of Conduct
   
   - [X] I agree to follow this project's [Code of Conduct](https://github.com/apache/airflow/blob/main/CODE_OF_CONDUCT.md)
   


-- 
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: commits-unsubscribe@airflow.apache.org.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [airflow] potiuk closed issue #27182: SSHOperator ignores cmd_timeout

Posted by GitBox <gi...@apache.org>.
potiuk closed issue #27182: SSHOperator ignores cmd_timeout
URL: https://github.com/apache/airflow/issues/27182


-- 
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: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [airflow] boring-cyborg[bot] commented on issue #27182: SSHOperator ignores cmd_timeout

Posted by GitBox <gi...@apache.org>.
boring-cyborg[bot] commented on issue #27182:
URL: https://github.com/apache/airflow/issues/27182#issuecomment-1286897700

   Thanks for opening your first issue here! Be sure to follow the issue template!
   


-- 
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: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [airflow] o-nikolas commented on issue #27182: SSHOperator ignores cmd_timeout

Posted by GitBox <gi...@apache.org>.
o-nikolas commented on issue #27182:
URL: https://github.com/apache/airflow/issues/27182#issuecomment-1290108803

   Thanks for working on this issue @punx120, I have assigned it to you :smile: 
   
   > Then timeout occurs, readq will be an empty list but there is no logic to handle that, and the code will keep reading forever. Not sure yet of the best way to handle this ...
   
   If `read`, `write`, `except` (the latter two are currently being thrown away with `_`) are *all* empty then you know you've hit your timeout case. Try adding such a condition to the hook code it should get you unblocked! :smiley:  
   


-- 
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: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [airflow] punx120 commented on issue #27182: SSHOperator ignores cmd_timeout

Posted by GitBox <gi...@apache.org>.
punx120 commented on issue #27182:
URL: https://github.com/apache/airflow/issues/27182#issuecomment-1291328907

   Ok done - I updated the PR and fix/added tests.


-- 
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: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [airflow] punx120 commented on issue #27182: SSHOperator ignores cmd_timeout

Posted by GitBox <gi...@apache.org>.
punx120 commented on issue #27182:
URL: https://github.com/apache/airflow/issues/27182#issuecomment-1289078039

   Actually after more testing - this is not enough - the timeout logic in hooks/ssh.py is "broken"
   
   ```
   while not channel.closed or channel.recv_ready() or channel.recv_stderr_ready():
               readq, _, _ = select([channel], [], [], timeout)
               for recv in readq:
   ```
   
   Then timeout occurs, `readq` will be an empty list but there is no logic to handle that, and the code will keep _reading_ forever. Not sure yet of the best way to handle this ...


-- 
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: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [airflow] punx120 commented on issue #27182: SSHOperator ignores cmd_timeout

Posted by GitBox <gi...@apache.org>.
punx120 commented on issue #27182:
URL: https://github.com/apache/airflow/issues/27182#issuecomment-1290422054

   Right, i'll give it a try an update the above PR


-- 
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: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org