Return for remote commander #581
Conversation
Remote script can return to his caller any Python value. Signed-off-by: Andrei Stepanov <astepano@redhat.com>
Protect SSH link between master & runner from messages built in
libs.
Signed-off-by: Andrei Stepanov <astepano@redhat.com>
Andrei-Stepanov
force-pushed
the
return_for_remote_commander
branch
from
6d3f382
to
ffdd2eb
Compare
| @@ -26,6 +26,13 @@ | |||
| import messenger as ms | |||
|
|
|||
|
|
|||
| logging.basicConfig(level=logging.DEBUG) | |||
| logger = logging.getLogger(__name__) | |||
| handler = logging.FileHandler("/tmp/remote_runner.log", "w", encoding=None, | |||
There was a problem hiding this comment.
How about get temp log file like below:
tempfile.mktemp(suffix='.log', prefix='remote_runner', dir=data_dir.get_tmp_dir()),
so that your log will not be overwrite.
|
@xutian : cannot use data_dir module in remote_runner as it is copied to remote VM. And it runs in its own, doesn't knowing anything about Avocado.
|
Signed-off-by: Andrei Stepanov <astepano@redhat.com>
Andrei-Stepanov
force-pushed
the
return_for_remote_commander
branch
from
5c5b718
to
8c71981
Compare
|
thanks your explain, update looks good to me. thanks @Andrei-Stepanov |
|
May I ask what are rules for acceptance of pull-requests? |
| fd_stdout = sys.stdout.fileno() # == 1 | ||
| fd_stderr = sys.stderr.fileno() # == 2 | ||
| orig_stdout = os.dup(fd_stdout) # Original stdout to master. | ||
| orig_stderr = os.dup(fd_stderr) # Original srderr to master. |
There was a problem hiding this comment.
Seems this orig_stderr not used
I'm not familiar with remote runner, so just check this patch code here, no more comment from me.
There was a problem hiding this comment.
Yes.
All SSH encoded base64 traffic goes through stdout.
Executed program/script has modified stdout/stderr.
stderr/stdout from remote_runner are endcoded and passed through original SSH stdout.
So.. in general stdout from SSH encapsulates stdout/stderr from program.
What you have pointed is not a bug. We can safely remove this variable. But...
I am sure that this code is a bit "tricky", so I just left this variable to represent what is really going here.
I can remove it. But it is safe to keep it for demonstrativeness. Do you want me to remove this variable ?
There was a problem hiding this comment.
Yest, it's really a tricky point, keep it is fine.
|
In fact, I have read your PR a few weeks ago. Only VM.commander and VM.remote_commander will use remote_runner.py. so if you can provide some test result or validation method to us next time, maybe you will get quick response. |
| @@ -726,7 +741,7 @@ def python_file_run_with_helper(self, test_path): | |||
| assert hasattr(module, "run") | |||
| run = getattr(module, "run") | |||
| helper = Helper(self) | |||
| run(helper) | |||
| return run(helper) | |||
|
|
|||
There was a problem hiding this comment.
return code is OK,
ACK commit 983d589
|
This PR got two ACK, so merged it. thanks @Andrei-Stepanov , @PandaWei |
No description provided.