search

ARM-software / devlib

Library for interaction with and instrumentation of remote devices.
Apache License 2.0
47 stars 78 forks source link

Fix push/pull semantic #563

Closed douglas-raillard-arm closed 2 years ago

douglas-raillard-arm commented 3 years ago

Ensure uniform handling of push/pull operations across connection types by removing the need for connections to interpret the paths they are given. With that PR, the connection is guaranteed that the destination does not exist but its parent folder does.

The cost of extra execute() is almost offset be memoizing SshConnection._get_sftp() in my setup. Other connection types will probably see a slowdown when pulling/pushing lots of small files.

Fixes https://github.com/ARM-software/devlib/issues/559

douglas-raillard-arm commented 3 years ago

TypeError(can only concatenate tuple (not "list") to tuple)

PR updated with the fix

douglas-raillard-arm commented 2 years ago

@marcbonnici @setrofim is there any blocker remaining for this PR ?

douglas-raillard-arm commented 2 years ago

This repeats for any further commands that are attempted on the target, have you seen anything like this on your end?

That is a strange error, I've never seen anything like that unfortunately. Maybe some sort of number of connection limit was reached ? The Administratively prohibited part makes me think it's a problem in the ssh server conf. I remember some people having issues with SFTP because OpenSSH does not ship with its SFTP plugin by default on some distro, installing an extra package is needed.

douglas-raillard-arm commented 2 years ago

@marcbonnici PR updated with fixed as_root value on via_temp=True path

marcbonnici commented 2 years ago

That is a strange error, I've never seen anything like that unfortunately. Maybe some sort of number of connection limit was reached ? The Administratively prohibited part makes me think it's a problem in the ssh server conf.

Ok trying out your latest version I cannot reproduce the error any more. In the latest update did you make any other changes apart from the via_temp parameter?

douglas-raillard-arm commented 2 years ago

In the latest update did you make any other changes apart from the via_temp parameter?

not AFAIR. Since this PR does not touch how the SFTP client is setup I don't think your previous error is related to this change. Change to paths passed to SFTP should only possibly result in file not founds and things like that.

marcbonnici commented 2 years ago

Since this PR does not touch how the SFTP client is setup I don't think your previous error is related to this change.

Ok good to know, for some reason I only ever hit it when testing this specific PR and when switching to master it magically went away again. But I can't see anything in the code that looks like it would be cause and I've ran through my tests another 2 times now with no issues so I think I'll chalk it up to a device quirk.