Fix regex and add TORQUE validation

xjules · Oct 26, 2023 · 4d20069 · 4d20069
1 parent c9cf211
commit 4d20069
Show file tree

Hide file tree

Showing 2 changed files with 44 additions and 26 deletions.
diff --git a/src/ert/config/queue_config.py b/src/ert/config/queue_config.py
@@ -99,7 +99,7 @@ def from_dict(cls, config_dict: ConfigDict) -> QueueConfig:
             selected_queue_system == QueueSystem.TORQUE
             and queue_options[QueueSystem.TORQUE]
         ):
-            _validate_torque_options(queue_options[QueueSystem.TORQUE])
+            _validate_queue_options(queue_options[QueueSystem.TORQUE], "TORQUE")
 
         if selected_queue_system == QueueSystem.LSF and queue_options[QueueSystem.LSF]:
             _validate_queue_options(queue_options[QueueSystem.LSF], "LSF")
@@ -148,21 +148,11 @@ def generate_dict(option_list: List[Tuple[str, str]]) -> Dict[str, List[str]]:
             )
 
 
-def _validate_torque_options(torque_options: List[Tuple[str, str]]) -> None:
-    for option_strings in torque_options:
-        option_name = option_strings[0]
-        option_value = option_strings[1]
-        if (
-            option_value != ""  # This is equivalent to the option not being set
-            and option_name == "MEMORY_PER_JOB"
-            and re.match("[0-9]+[mg]b", option_value) is None
-        ):
-            raise ConfigValidationError(
-                f"The value '{option_value}' is not valid for the Torque option "
-                "MEMORY_PER_JOB, it must be of "
-                "the format '<integer>mb' or '<integer>gb'."
-            )
-
+memory_options: Mapping[str, List[str]] = {
+    "LSF": [],
+    "SLURM": [],
+    "TORQUE": ["MEMORY_PER_JOB"],
+}
 
 string_options: Mapping[str, List[str]] = {
     "LSF": [
@@ -188,6 +178,14 @@ def _validate_torque_options(torque_options: List[Tuple[str, str]]) -> None:
         "INCLUDE_HOST",
         "EXCLUDE_HOST",
     ],
+    "TORQUE": [
+        "QSUB_CMD",
+        "QSTAT_CMD",
+        "QDEL_CMD",
+        "QSTAT_OPTIONS",
+        "QUEUE",
+        "DEBUG_OUTPUT",
+    ],
 }
 
 positive_int_options: Mapping[str, List[str]] = {
@@ -200,21 +198,26 @@ def _validate_torque_options(torque_options: List[Tuple[str, str]]) -> None:
         "MEMORY_PER_CPU",
         "MAX_RUNNING",
     ],
+    "TORQUE": [
+        "NUM_NODES",
+        "NUM_CPUS_PER_NODE",
+    ],
 }
 
 float_options: Mapping[str, List[str]] = {
     "LSF": [
         "SUBMIT_SLEEP",
     ],
     "SLURM": [
-        "SUBMIT_SLEEP",
         "SQUEUE_TIMEOUT",
     ],
+    "TORQUE": ["SUBMIT_SLEEP", "QUEUE_QUERY_TIMEOUT"],
 }
 
 bool_options: Mapping[str, List[str]] = {
     "LSF": ["DEBUG_OUTPUT"],
     "SLURM": [],
+    "TORQUE": [],
 }
 
 
@@ -224,22 +227,34 @@ def _validate_queue_options(
     for option_strings in queue_options:
         option_name = option_strings[0]
         option_value = option_strings[1]
+        if (
+            option_value != ""  # This is equivalent to the option not being set
+            and option_name in memory_options[queue_type]
+            and re.match("[0-9]+[mg]b", option_value) is None
+        ):
+            raise ConfigValidationError(
+                f"The value '{option_value}' is not valid for "
+                f"the {queue_type} option "
+                "MEMORY_PER_JOB, it must be of "
+                "the format '<integer>mb' or '<integer>gb'."
+            )
         if option_name in string_options[queue_type] and not isinstance(
             option_value, str
         ):
             raise ConfigValidationError(
-                f"The value '{option_value}' is not valid string for the {queue_type} option "
+                f"The value '{option_value}' is not valid string "
+                f"for the {queue_type} option "
                 f"{option_name}, it must be of string type."
             )
         if option_name in float_options[queue_type] and (
-            re.match(r"\d+(\.\d+)?", option_value) is None and option_value != ""
+            re.match(r"^\d+(\.\d+)?$", option_value) is None and option_value != ""
         ):
             raise ConfigValidationError(
                 f"The value '{option_value}' is not valid integer or float"
                 f" for the {queue_type} option {option_name}."
             )
         if option_name in positive_int_options[queue_type] and (
-            re.match(r"\d+", option_value) is None and option_value != ""
+            re.match(r"^\d+$", option_value) is None and option_value != ""
         ):
             raise ConfigValidationError(
                 f"The value '{option_value}' is not valid positive integer "

diff --git a/tests/unit_tests/config/test_queue_config.py b/tests/unit_tests/config/test_queue_config.py
@@ -207,16 +207,19 @@ def test_initializing_empty_config_values(queue_system, queue_system_option):
 
 @pytest.mark.usefixtures("use_tmpdir")
 @pytest.mark.parametrize(
-    "queue_system, queue_system_option",
-    [("LSF", "LSF_SERVER")],
+    "queue_system, queue_option, queue_value, err_msg",
+    [
+        ("LSF", "BJOBS_TIMEOUT", "-3", "is not valid positive integer"),
+        ("SLURM", "SQUEUE_TIMEOUT", "5a", "is not valid integer or float"),
+        ("TORQUE", "NUM_NODES", "3.5", "is not valid positive integer"),
+    ],
 )
-def test_wrong_config_types(queue_system, queue_system_option):
+def test_wrong_config_option_types(queue_system, queue_option, queue_value, err_msg):
     filename = "config.ert"
     with open(filename, "w", encoding="utf-8") as f:
         f.write("NUM_REALIZATIONS 1\n")
         f.write(f"QUEUE_SYSTEM {queue_system}\n")
-        f.write(f"QUEUE_OPTION {queue_system} {queue_system_option}\n")
-        f.write(f"QUEUE_OPTION {queue_system} SUBMIT_SLEEP nan\n")
+        f.write(f"QUEUE_OPTION {queue_system} {queue_option} {queue_value}\n")
 
-    with pytest.raises(ConfigValidationError, match="is not valid integer or float"):
+    with pytest.raises(ConfigValidationError, match=err_msg):
         ErtConfig.from_file(filename)