Resolved some issues where execution could not be voluntarily stopped

[zuohanxu]

No description provided.

[zuohanxu]

@richtja @clebergnu Some commands cannot be automatically stopped during execution. We can add the parameter timeout to terminate the command. What do you think

[clebergnu]

Hi @zuohanxu,

First of all, I apologize for taking so long to answer this.

Now, the idea is good, but I'm afraid there's something missing. I wrote this reproducer:

import time

from avocado.utils.ssh import Session


with Session("host", user="root", key="/path/to/key") as s:
    # baseline case
    time_start = time.monotonic()
    s.cmd("sleep 10")
    total_time = time.monotonic() - time_start
    print(total_time)
    assert total_time >= 10

    # check of timeout enforcement
    time_start = time.monotonic()
    s.cmd("sleep 10", timeout=1)
    total_time = time.monotonic() - time_start
    print(total_time)
    assert total_time <= 10

In my case, the second assertion fails. I suspect process.run() is either failing to enforce the timeout or the way it enforces the timeout is not sufficiently effective for an ssh command.

Let me know if you can reproduce what I found, and we can work together on a solution.

[zuohanxu]

Hi @clebergnu ,
Thank you for reminding me, I also found this problem,process.run() does not take effect for ssh command,so I changed the way to implement, and I also verified that this scheme is feasible, the following is my code implementation, if you think it is OK, then I will submit the modification

def cmd(self, command, ignore_status=True, timeout=None):
       try:
            if timeout:
                command_argument = f"timeout --foreground {timeout} {command}"
            else:
                command_argument = command
            return process.run(
                self.get_raw_ssh_command(command_argument),
                ignore_status=ignore_status,
            )
        except process.CmdError as exc:
            if exc.result.exit_status == 255:
                exc.additional_text = "SSH connection failed"
            else:
                exc.additional_text = f"Command '{command}' failed"
                exc.stderr = exc.result.stderr
                exc.stdout = exc.result.stdout
            raise exc

[zuohanxu]

@clebergnu @richtja Do you think this idea is OK with me? If you have any questions, please help me point out, thank you

[clebergnu]

@clebergnu @richtja Do you think this idea is OK with me? If you have any questions, please help me point out, thank you

Hi @zuohanxu ,

Yes, I think your approach is sound. I'll be on the lookout for the actual implementation.

Thanks!

[zuohanxu]

Hi @clebergnu
I have submitted my modification again and it has passed the verification. If there is no problem, could you help me merge it

[zuohanxu]

Hi @clebergnu @richtja
I fix CI test,Can you check the code for me.
Thanks!

[richtja]

Hello @zuohanxu,

thank you for your recent contributions. The code LGTM and I'm quite satisfied with the changes you've made. I do have a few suggestions concerning the organization of commits, though.

Firstly, could you please exclude the commit d35b1e5? It appears to be a remnant from the development phase and is no longer necessary.

Additionally, I'd like to propose combining the Modify static check commits with the ones that alter the behavior. This way, we can maintain a coherent and seamless progression of changes. My preference is to retain only the commits fa4ec66 and be04d70, as they adhere to the appropriate style.

If you will need any help with that, don't hesitate to ask.

Thank you once again for your valuable contributions.

[zuohanxu]

Hi @clebergnu
Thank you for your review. I have modified my submission record

[clebergnu]

@zuohanxu I sent a joint-effort version as #5775. I'm closing this PR because of that. Feel free to re-open or continue the discussion on the new PR.