-
Notifications
You must be signed in to change notification settings - Fork 6
lib/host: Avoid using identical source and destination paths in execute_script scp #334
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?
lib/host: Avoid using identical source and destination paths in execute_script scp #334
Conversation
lib/host.py
Outdated
@@ -201,13 +201,13 @@ def execute_script(self, script_contents, shebang='sh', simple_output=True): | |||
script.write('#!/usr/bin/env ' + shebang + '\n') | |||
script.write(script_contents) | |||
script.flush() | |||
self.scp(script.name, script.name) | |||
self.scp(script.name, "/tmp/" + script.name.split('/')[-1]) |
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.
better use os.path.basneame
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.
thanks, made the change.
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.
Ping @ydirson
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.
Better, but computing the same path 3 times is suboptimal also for reading :)
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.
Instead of computing paths each time, you could eventually use mktemp
on the remote:
(Like in a fixture here)
@pytest.fixture(scope="module")
def temp_dir(running_unix_vm):
vm = running_unix_vm
tempdir = vm.ssh("mktemp -d")
yield tempdir
# teardown
vm.ssh(f"rm -r {tempdir}")
Or something like this for local use:
import tempfile
### Either using contexts
with tempfile.TemporaryDirectory() as tmpdirname:
print('created temporary directory', tmpdirname)
### Either controlling the lifetime manually
temp_dir = tempfile.TemporaryDirectory()
print(temp_dir.name)
temp_dir.cleanup()
This is for dirs but is quite the same for files.
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.
I like this fixture :)
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.
Straight up borrowed from #324 😁
It could be great to have this kind of generic fixtures available for all tests 👀 (eventually platform agnostic unlike the one I suggested)
…te_script scp Using the same path for both source and destination may cause scp to fail, especially when the source path (e.g. a temp path on macOS) does not exist on the remote system. Thus, resorting to use `/tmp/` and random name from `sctipt.name` on destination. Signed-off-by: Rushikesh Jadhav <[email protected]>
87dab23
to
9414d56
Compare
Using the same path for both source and destination may cause scp to fail, especially when the source path (e.g. a temp path on macOS) does not exist on the remote system. Thus, resorting to use
/tmp/
and random name fromsctipt.name
on destination.