From 644a36caa7512da62cd10e50a38ad0ed819a4a4c Mon Sep 17 00:00:00 2001 From: Bo Peng Date: Sun, 4 Feb 2024 00:03:48 -0600 Subject: [PATCH 1/5] Write offending script to stderr (#i530) --- src/sos/actions.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/sos/actions.py b/src/sos/actions.py index ac571c5d4..33af0ae3c 100644 --- a/src/sos/actions.py +++ b/src/sos/actions.py @@ -589,6 +589,11 @@ def run(self, **kwargs): p = subprocess.Popen(cmd, shell=True, stderr=se, stdout=so) ret = p.wait() + + if ret != 0: + # write an error message to stderr + se.write('\nError occured when executing the following script:\n\n{self.script}\n\n') + if so is not None and so != subprocess.DEVNULL: so.close() if se is not None and se != subprocess.DEVNULL: From 9d74bccc50c10472007ce6df01bd0be7c8f4ca3f Mon Sep 17 00:00:00 2001 From: Gao Wang Date: Sun, 4 Feb 2024 08:09:49 -0500 Subject: [PATCH 2/5] Directly modify debug script path #1533 --- src/sos/actions.py | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/src/sos/actions.py b/src/sos/actions.py index 33af0ae3c..9f9ee538a 100644 --- a/src/sos/actions.py +++ b/src/sos/actions.py @@ -411,8 +411,10 @@ def run(self, **kwargs): else: raise RuntimeError(f"Unacceptable interpreter {self.interpreter}") + debug_script_path = os.path.dirname(os.path.abspath(kwargs["stderr"])) if ("stderr" in kwargs and kwargs["stderr"] is not False and + os.path.isdir(os.path.dirname(os.path.abspath(kwargs["stderr"])))) else env.exec_dir debug_script_file = os.path.join( - env.exec_dir, + debug_script_path, f'{env.sos_dict["step_name"]}_{env.sos_dict["_index"]}_{str(uuid.uuid4())[:8]}{self.suffix}', ) # with open(debug_script_file, 'w') as sfile: @@ -587,13 +589,8 @@ def run(self, **kwargs): se = subprocess.DEVNULL p = subprocess.Popen(cmd, shell=True, stderr=se, stdout=so) - ret = p.wait() - if ret != 0: - # write an error message to stderr - se.write('\nError occured when executing the following script:\n\n{self.script}\n\n') - if so is not None and so != subprocess.DEVNULL: so.close() if se is not None and se != subprocess.DEVNULL: From ffeac9cb300a3e247c48598649c5118014a8a4a2 Mon Sep 17 00:00:00 2001 From: Gao Wang Date: Sun, 4 Feb 2024 10:00:30 -0500 Subject: [PATCH 3/5] Further address comment on #1530 --- src/sos/actions.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/sos/actions.py b/src/sos/actions.py index 9f9ee538a..dd624ef71 100644 --- a/src/sos/actions.py +++ b/src/sos/actions.py @@ -590,6 +590,9 @@ def run(self, **kwargs): p = subprocess.Popen(cmd, shell=True, stderr=se, stdout=so) ret = p.wait() + if ret != 0: + # write an error message to stderr + se.write(f'\nError occured when executing the following code chunk, saved to script:\n{debug_script_file}\n\n') if so is not None and so != subprocess.DEVNULL: so.close() From 60c9c633026209bcf2eaa8f9714ab7af9908c287 Mon Sep 17 00:00:00 2001 From: Bo Peng Date: Sun, 4 Feb 2024 10:40:49 -0600 Subject: [PATCH 4/5] Output expanded script during task execution --- src/sos/actions.py | 21 ++++++++++++++++++--- 1 file changed, 18 insertions(+), 3 deletions(-) diff --git a/src/sos/actions.py b/src/sos/actions.py index dd624ef71..8986184a7 100644 --- a/src/sos/actions.py +++ b/src/sos/actions.py @@ -411,12 +411,14 @@ def run(self, **kwargs): else: raise RuntimeError(f"Unacceptable interpreter {self.interpreter}") - debug_script_path = os.path.dirname(os.path.abspath(kwargs["stderr"])) if ("stderr" in kwargs and kwargs["stderr"] is not False and + debug_script_path = os.path.dirname(os.path.abspath(kwargs["stderr"])) if ("stderr" in kwargs and kwargs["stderr"] is not False and os.path.isdir(os.path.dirname(os.path.abspath(kwargs["stderr"])))) else env.exec_dir debug_script_file = os.path.join( debug_script_path, f'{env.sos_dict["step_name"]}_{env.sos_dict["_index"]}_{str(uuid.uuid4())[:8]}{self.suffix}', ) + debug_script_msg = f'\n>>> START SCRIPT ({debug_script_file}) <<<\n\n{self.script.strip()}\n\n>>> END SCRIPT <<<\n' + # with open(debug_script_file, 'w') as sfile: # sfile.write(self.script) # env.log_to_file('ACTION', self.script) @@ -525,6 +527,7 @@ def run(self, **kwargs): ret = pexpect_run(cmd.strip()) elif "__std_out__" in env.sos_dict and "__std_err__" in env.sos_dict: + # task execution if "stdout" in kwargs or "stderr" in kwargs: if "stdout" in kwargs: if kwargs["stdout"] is False: @@ -549,6 +552,9 @@ def run(self, **kwargs): p = subprocess.Popen(cmd, shell=True, stderr=se, stdout=so) ret = p.wait() + if ret != 0: + se.write(debug_script_msg.encode()) + if so != subprocess.DEVNULL: so.close() if se != subprocess.DEVNULL: @@ -559,6 +565,9 @@ def run(self, **kwargs): "ab") as se: p = subprocess.Popen(cmd, shell=True, stderr=se, stdout=so) ret = p.wait() + + if ret != 0: + se.write(debug_script_msg.encode()) else: p = subprocess.Popen( cmd, @@ -567,6 +576,9 @@ def run(self, **kwargs): stdout=subprocess.DEVNULL, ) ret = p.wait() + + if ret != 0: + sys.stderr.write(debug_script_msg) else: if "stdout" in kwargs: if kwargs["stdout"] is False: @@ -590,9 +602,12 @@ def run(self, **kwargs): p = subprocess.Popen(cmd, shell=True, stderr=se, stdout=so) ret = p.wait() + if ret != 0: - # write an error message to stderr - se.write(f'\nError occured when executing the following code chunk, saved to script:\n{debug_script_file}\n\n') + if se: + se.write(debug_script_msg.encode()) + else: + sys.stderr.write(debug_script_msg) if so is not None and so != subprocess.DEVNULL: so.close() From 93f77678990421dcf5e2bdb55492bdc3a9eef979 Mon Sep 17 00:00:00 2001 From: Bo Peng Date: Sun, 4 Feb 2024 11:04:30 -0600 Subject: [PATCH 5/5] Fix pylint --- .github/linters/.python-lint | 1 + src/sos/eval.py | 1 - src/sos/singularity/client.py | 2 +- src/sos/task_engines.py | 3 ++- src/sos/tasks.py | 6 ++++-- 5 files changed, 8 insertions(+), 5 deletions(-) diff --git a/.github/linters/.python-lint b/.github/linters/.python-lint index ffb031d00..566d2b5ff 100755 --- a/.github/linters/.python-lint +++ b/.github/linters/.python-lint @@ -105,6 +105,7 @@ disable=abstract-method, missing-module-docstring, no-member, no-name-in-module, + no-self-use, no-value-for-parameter, pointless-statement, protected-access, diff --git a/src/sos/eval.py b/src/sos/eval.py index ec3c96c17..9924d6c69 100644 --- a/src/sos/eval.py +++ b/src/sos/eval.py @@ -508,4 +508,3 @@ def analyze_global_statements(global_stmt): f"Variable {key} cannot be defined in global section because it cannot be pickled to workers." ) from e return global_def, global_vars - diff --git a/src/sos/singularity/client.py b/src/sos/singularity/client.py index 719bd5737..9063c180f 100644 --- a/src/sos/singularity/client.py +++ b/src/sos/singularity/client.py @@ -332,7 +332,7 @@ def run(self, for opt in ['nv', 'nvccli', 'disable_cache', 'nohttps', 'nonet', 'vm_err', 'writable', 'writable_tmpfs', 'vm', 'uts', 'userns', 'rocm', 'pid', 'passphrase', 'no_mark', 'no_privs', 'no_init', 'no_https', 'no_home', 'net', - 'keep_privs', 'fakeroot'', disable_cache', 'containall', 'contain', + 'keep_privs', 'fakeroot', 'disable_cache', 'containall', 'contain', 'compat', 'cleanenv', 'allow_setuid']: if opt in kwargs and kwargs[opt]: exec_opts.append('--' + opt.replace('_', '-')) diff --git a/src/sos/task_engines.py b/src/sos/task_engines.py index 133fa79d5..2462ae39c 100644 --- a/src/sos/task_engines.py +++ b/src/sos/task_engines.py @@ -376,7 +376,8 @@ def submit_task(self, task_id): self.engine_ready.wait() # submit tasks simply add task_id to pending task list - with threading.Lock(): + lock = threading.Lock() + with lock: # if already in # if task_id in self.running_tasks or task_id in self.pending_tasks: # self.notify_controller('{} ``{}``'.format(task_id, self.task_status[task_id])) diff --git a/src/sos/tasks.py b/src/sos/tasks.py index 1122b491c..7dfe6204c 100644 --- a/src/sos/tasks.py +++ b/src/sos/tasks.py @@ -479,6 +479,8 @@ def add_outputs(self, keep_result=False): with fasteners.InterProcessLock(os.path.join(env.temp_dir, self.task_id + ".lck")): with open(self.task_file, "r+b") as fh: header = self._read_header(fh) + result = '' + signature = '' if header.result_size != 0: if not keep_result: result_size = 0 @@ -514,9 +516,9 @@ def add_outputs(self, keep_result=False): fh.write(stdout) if stderr: fh.write(stderr) - if result_size > 0: + if result_size > 0 and result: fh.write(result) - if signature_size > 0: + if signature_size > 0 and signature: fh.write(signature) def add_result(self, result: dict = {}):