Skip to content

SSHOperator ignores cmd_timeout #27182

Closed
#27184
Closed
SSHOperator ignores cmd_timeout#27182
#27184
Assignees
punx120
Labels
area:providerskind:bugThis is a clearly a bug

Description

@punx120
punx120
opened on Oct 21, 2022

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?

  • Yes I am willing to submit a PR!

Code of Conduct

Activity

  1. punx120
    added
    area:providers
    kind:bugThis is a clearly a bug
    on Oct 21, 2022

  2. boring-cyborg commented on Oct 21, 2022

    @boring-cyborg
    boring-cyborgBot
    on Oct 21, 2022

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

  3. punx120 commented on Oct 24, 2022

    @punx120
    punx120
    on Oct 24, 2022
    ContributorAuthor

    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 ...

  4. o-nikolas
    added
    provider:Protocols
    on Oct 25, 2022
  5. o-nikolas
    assigned
    punx120
    on Oct 25, 2022

  6. o-nikolas commented on Oct 25, 2022

    @o-nikolas
    o-nikolas
    on Oct 25, 2022
    Contributor

    Thanks for working on this issue @punx120, I have assigned it to you 😄

    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! 😃

  7. punx120 commented on Oct 25, 2022

    @punx120
    punx120
    on Oct 25, 2022
    ContributorAuthor

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

  8. punx120 commented on Oct 26, 2022

    @punx120
    punx120
    on Oct 26, 2022
    ContributorAuthor

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

  9. punx120
    added a commit that references this issue on Oct 26, 2022

    SSHOperator ignores cmd_timeout (apache#27182)

    7b11450
  10. potiuk
    closed this as completedin #27184on Nov 7, 2022
  11. potiuk
    added a commit that references this issue on Nov 7, 2022

    SSHOperator ignores cmd_timeout (#27182) (#27184)

    dc760b4
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

area:providerskind:bugThis is a clearly a bug

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions