Provide configurable ability for locally executed scripts to abort ea…

…rly (#528)

- Future work: add support for remote exec version of this

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

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)

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
         -------

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
+    }
+}

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"
     }
 }

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"
     }
 }

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",
+    ]

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, "", "")