diff --git a/mlos_bench/mlos_bench/config/schemas/services/local/local-exec-service-subschema.json b/mlos_bench/mlos_bench/config/schemas/services/local/local-exec-service-subschema.json index 830094e0402..9e085223191 100644 --- a/mlos_bench/mlos_bench/config/schemas/services/local/local-exec-service-subschema.json +++ b/mlos_bench/mlos_bench/config/schemas/services/local/local-exec-service-subschema.json @@ -17,6 +17,14 @@ "allOf": [ { "$ref": "../common-defs-subschemas.json#/$defs/temp_dir_config" + }, + { + "properties": { + "abort_on_error": { + "description": "Whether or not to abort immediately when a script line returns an errorcode.", + "type": "boolean" + } + } } ], "unevaluatedProperties": false diff --git a/mlos_bench/mlos_bench/services/local/local_exec.py b/mlos_bench/mlos_bench/services/local/local_exec.py index b2b48ce6f01..52e35b0fcd7 100644 --- a/mlos_bench/mlos_bench/services/local/local_exec.py +++ b/mlos_bench/mlos_bench/services/local/local_exec.py @@ -85,12 +85,12 @@ def __init__(self, An optional parent service that can provide mixin functions. """ super().__init__(config, global_config, parent) + self.abort_on_error = self.config.get("abort_on_error", True) self.register([self.local_exec]) def local_exec(self, script_lines: Iterable[str], env: Optional[Mapping[str, "TunableValue"]] = None, - cwd: Optional[str] = None, - return_on_error: bool = False) -> Tuple[int, str, str]: + cwd: Optional[str] = None) -> Tuple[int, str, str]: """ Execute the script lines from `script_lines` in a local process. @@ -104,9 +104,6 @@ def local_exec(self, script_lines: Iterable[str], cwd : str Work directory to run the script at. If omitted, use `temp_dir` or create a temporary dir. - return_on_error : bool - If True, stop running script lines on first non-zero return code. - The default is False. Returns ------- @@ -122,7 +119,7 @@ def local_exec(self, script_lines: Iterable[str], (return_code, stdout, stderr) = self._local_exec_script(line, env, temp_dir) stdout_list.append(stdout) stderr_list.append(stderr) - if return_code != 0 and return_on_error: + if return_code != 0 and self.abort_on_error: break stdout = "".join(stdout_list) diff --git a/mlos_bench/mlos_bench/services/types/local_exec_type.py b/mlos_bench/mlos_bench/services/types/local_exec_type.py index 5dbd47d52db..1a3808bb617 100644 --- a/mlos_bench/mlos_bench/services/types/local_exec_type.py +++ b/mlos_bench/mlos_bench/services/types/local_exec_type.py @@ -27,8 +27,7 @@ class SupportsLocalExec(Protocol): def local_exec(self, script_lines: Iterable[str], env: Optional[Mapping[str, TunableValue]] = None, - cwd: Optional[str] = None, - return_on_error: bool = False) -> Tuple[int, str, str]: + cwd: Optional[str] = None) -> Tuple[int, str, str]: """ Execute the script lines from `script_lines` in a local process. @@ -42,9 +41,6 @@ def local_exec(self, script_lines: Iterable[str], cwd : str Work directory to run the script at. If omitted, use `temp_dir` or create a temporary dir. - return_on_error : bool - If True, stop running script lines on first non-zero return code. - The default is False. Returns ------- diff --git a/mlos_bench/mlos_bench/tests/config/schemas/services/test-cases/bad/invalid/local_exec_service-invalid-abort_on_error.jsonc b/mlos_bench/mlos_bench/tests/config/schemas/services/test-cases/bad/invalid/local_exec_service-invalid-abort_on_error.jsonc new file mode 100644 index 00000000000..afd2a804c75 --- /dev/null +++ b/mlos_bench/mlos_bench/tests/config/schemas/services/test-cases/bad/invalid/local_exec_service-invalid-abort_on_error.jsonc @@ -0,0 +1,7 @@ +{ + "class": "mlos_bench.services.local.local_exec.LocalExecService", + + "config": { + "abort_on_error": "false" // wrong type + } +} diff --git a/mlos_bench/mlos_bench/tests/config/schemas/services/test-cases/good/full/local_exec_service-full.jsonc b/mlos_bench/mlos_bench/tests/config/schemas/services/test-cases/good/full/local_exec_service-full.jsonc index 58b6c9f5cf8..309d8400902 100644 --- a/mlos_bench/mlos_bench/tests/config/schemas/services/test-cases/good/full/local_exec_service-full.jsonc +++ b/mlos_bench/mlos_bench/tests/config/schemas/services/test-cases/good/full/local_exec_service-full.jsonc @@ -1,9 +1,10 @@ { - "$schema": "https://raw.githubusercontent.com/microsoft/MLOS/main/mlos_bench/mlos_bench/config/schemas/services/service-schema.json", + //"$schema": "https://raw.githubusercontent.com/microsoft/MLOS/main/mlos_bench/mlos_bench/config/schemas/services/service-schema.json", "class": "mlos_bench.services.local.local_exec.LocalExecService", "description": "descriptive text", "config": { + "abort_on_error": true, "temp_dir": "/tmp" } } diff --git a/mlos_bench/mlos_bench/tests/config/schemas/services/test-cases/good/partial/local_exec_service-partial.jsonc b/mlos_bench/mlos_bench/tests/config/schemas/services/test-cases/good/partial/local_exec_service-partial.jsonc index 43df536454e..ea296890521 100644 --- a/mlos_bench/mlos_bench/tests/config/schemas/services/test-cases/good/partial/local_exec_service-partial.jsonc +++ b/mlos_bench/mlos_bench/tests/config/schemas/services/test-cases/good/partial/local_exec_service-partial.jsonc @@ -2,6 +2,7 @@ "class": "mlos_bench.services.LocalExecService", "config": { + "abort_on_error": false, "temp_dir": "/tmp" } } diff --git a/mlos_bench/mlos_bench/tests/services/local/local_exec_test.py b/mlos_bench/mlos_bench/tests/services/local/local_exec_test.py index b637cf8f819..a7fa5951fde 100644 --- a/mlos_bench/mlos_bench/tests/services/local/local_exec_test.py +++ b/mlos_bench/mlos_bench/tests/services/local/local_exec_test.py @@ -50,7 +50,10 @@ def local_exec_service() -> LocalExecService: """ Test fixture for LocalExecService. """ - return LocalExecService(parent=ConfigPersistenceService()) + config = { + "abort_on_error": True, + } + return LocalExecService(config, parent=ConfigPersistenceService()) def test_resolve_script(local_exec_service: LocalExecService) -> None: @@ -59,8 +62,8 @@ def test_resolve_script(local_exec_service: LocalExecService) -> None: """ script = "os/linux/runtime/scripts/local/generate_kernel_config_script.py" script_abspath = local_exec_service.config_loader_service.resolve_path(script) - orig_cmdline = f". env.sh && {script}" - expected_cmdline = f". env.sh && {script_abspath}" + orig_cmdline = f". env.sh && {script} --input foo" + expected_cmdline = f". env.sh && {script_abspath} --input foo" subcmds_tokens = split_cmdline(orig_cmdline) # pylint: disable=protected-access subcmds_tokens = [local_exec_service._resolve_cmdline_script_path(subcmd_tokens) for subcmd_tokens in subcmds_tokens] @@ -168,3 +171,33 @@ def test_run_script_fail(local_exec_service: LocalExecService) -> None: (return_code, stdout, _stderr) = local_exec_service.local_exec(["foo_bar_baz hello"]) assert return_code != 0 assert stdout.strip() == "" + + +def test_run_script_middle_fail_abort(local_exec_service: LocalExecService) -> None: + """ + Try to run a series of commands, one of which fails, and abort early. + """ + (return_code, stdout, _stderr) = local_exec_service.local_exec([ + "echo hello", + "cmd /c 'exit 1'" if sys.platform == 'win32' else "false", + "echo world", + ]) + assert return_code != 0 + assert stdout.strip() == "hello" + + +def test_run_script_middle_fail_pass(local_exec_service: LocalExecService) -> None: + """ + Try to run a series of commands, one of which fails, but let it pass. + """ + local_exec_service.abort_on_error = False + (return_code, stdout, _stderr) = local_exec_service.local_exec([ + "echo hello", + "cmd /c 'exit 1'" if sys.platform == 'win32' else "false", + "echo world", + ]) + assert return_code == 0 + assert stdout.splitlines() == [ + "hello", + "world", + ] diff --git a/mlos_bench/mlos_bench/tests/services/local/mock/mock_local_exec_service.py b/mlos_bench/mlos_bench/tests/services/local/mock/mock_local_exec_service.py index 503c5e1956d..750538d26b0 100644 --- a/mlos_bench/mlos_bench/tests/services/local/mock/mock_local_exec_service.py +++ b/mlos_bench/mlos_bench/tests/services/local/mock/mock_local_exec_service.py @@ -32,6 +32,5 @@ def __init__(self, config: Optional[Dict[str, Any]] = None, def local_exec(self, script_lines: Iterable[str], env: Optional[Mapping[str, "TunableValue"]] = None, - cwd: Optional[str] = None, - return_on_error: bool = False) -> Tuple[int, str, str]: + cwd: Optional[str] = None) -> Tuple[int, str, str]: return (0, "", "")