Skip to content

Conversation

@olmanqj
Copy link
Contributor

@olmanqj olmanqj commented Oct 9, 2025

Added option use_legacy_protocol:bool to linux.copy and utils.copy_to_dir to use the legacy SCP protocol for file transfers instead of the SFTP protocol. Forcing the use of the SCP protocol may be necessary for servers that do not implement SFTP, for backwards compatibility. In essence is to pass -O option to SCP command.

Copy link
Owner

@Rahix Rahix left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi, thanks a lot for the contribution!

Yeah, I can see why this would be needed. However, I am wondering if it wouldn't make more sense to make this an attribute of the machine-class you are copying to? Then testcases don't have to know whether they need to set use_legacy_protocol, but instead automatically do the right thing depending on the machine that it is copying to.

So essentially, doing this in _scp_copy():

use_legacy_protocol = getattr(remote_path.host, "requires_legacy_scp", False)
if use_legacy_protocol:
    scp_command += ["-O"]

And then you just set requires_legacy_scp = True inside you machine class configuration for machines that need it.

What do you think?

@Rahix Rahix added the feature Enhancement/New feature label Oct 10, 2025
@olmanqj
Copy link
Contributor Author

olmanqj commented Oct 10, 2025

Hi, thanks a lot for the contribution!

Yeah, I can see why this would be needed. However, I am wondering if it wouldn't make more sense to make this an attribute of the machine-class you are copying to? Then testcases don't have to know whether they need to set use_legacy_protocol, but instead automatically do the right thing depending on the machine that it is copying to.

So essentially, doing this in _scp_copy():

use_legacy_protocol = getattr(remote_path.host, "requires_legacy_scp", False)
if use_legacy_protocol:
    scp_command += ["-O"]

And then you just set requires_legacy_scp = True inside you machine class configuration for machines that need it.

What do you think?

Sure thing, that sounds better :)

Copy link
Owner

@Rahix Rahix left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great, thank you very much!

@Rahix Rahix merged commit fa7d406 into Rahix:master Oct 14, 2025
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

feature Enhancement/New feature

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants