-
Notifications
You must be signed in to change notification settings - Fork 234
Feature: client: Add scp multifile support #1769
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Feature: client: Add scp multifile support #1769
Conversation
f6b4349 to
1cebc8b
Compare
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #1769 +/- ##
========================================
+ Coverage 45.1% 45.2% +0.1%
========================================
Files 172 172
Lines 13618 13626 +8
========================================
+ Hits 6144 6163 +19
+ Misses 7474 7463 -11
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
|
I would like to update tests, but I have no idea where to start. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can pull out bb72bc0 into a separate PR so we can merge that early.
labgrid/remote/client.py
Outdated
| subparser.add_argument("src", help="source path (use :dir/file for remote side)") | ||
| subparser.add_argument("dst", help="destination path (use :dir/file for remote side)") | ||
| subparser.add_argument("--recursive", "-r", action="store_true", help="copy recursive") | ||
| subparser.add_argument( | ||
| "files", nargs="+", metavar="SRC/DST", help="source and destination path (use :dir/file for remote side)" | ||
| ) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You should be able to just use nargs="+" for src and keep the separate dst argument.
| "-S", self._ssh, | ||
| "-F", "none", | ||
| "-o", f"ControlPath={self.control.replace('%', '%%')}", | ||
| src, dst, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can use *src to avoid the insert loop below.
| @step(args=['src', 'dst']) | ||
| def scp(self, *, src, dst): | ||
| @step(args=['src', 'dst', 'recursive']) | ||
| def scp(self, *, src, dst, recursive=False): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We need to consider backwards compatibility here. So, if src is a string, wrap it in a list.
labgrid/driver/sshdriver.py
Outdated
| if src.startswith(':'): | ||
| src = '_' + src | ||
| if dst.startswith(':'): | ||
| if all([f.startswith(':') for f in src]): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You need to check that either all src elements start with ':' or none of them do.
So perhaps first something like
remote_src = [f.startswith(':') for f in src]
if any(remote_src) != all(remote_src):
raise ValueError("All sources must be consistently local or remote (start with :)")before the other checks?
Use In the test, make two directories via in the tmpdir fixture and transfer beween them. |
1cebc8b to
edfc3c3
Compare
70e0c94 to
3f312be
Compare
Users are accustomed to scp having an option for copying recursively as well as to accept multiple source files. Meet user expectation by adding well known `-r` option and support for multiple source files. Signed-off-by: Sebastian Gross <[email protected]>
Signed-off-by: Sebastian Gross <[email protected]>
Use features of multifile scp driver Signed-off-by: Sebastian Gross <[email protected]>
Signed-off-by: Sebastian Gross <[email protected]>
3f312be to
1b2997c
Compare
Description
Implement
-rand[SOURCES...] [DESTINATION]forscpCollegues requested this feature since it matches the expectation on how to use the widely known
scpcommand.Checklist
to Target
from Target