Merge pull request #2893 from fractal-analytics-platform/2892-fix-unhandled-error-with-slurmconfigextra_lines

Make `extra_lines` a non-optional list

Author: tcompa

CHANGELOG.md

@@ -10,6 +10,7 @@ TBD
 \#2877
 \#2882
 \#2884
+\#2893
 
 # 2.16.6
 

fractal_server/runner/config/_slurm.py

@@ -3,6 +3,7 @@
 from pydantic import AfterValidator
 from pydantic import BaseModel
 from pydantic import ConfigDict
+from pydantic import Field
 from pydantic.types import PositiveInt
 
 from fractal_server.runner.config.slurm_mem_to_MB import slurm_mem_to_MB
@@ -44,7 +45,7 @@ class _SlurmConfigSet(BaseModel):
     nodelist: NonEmptyStr | None = None
     time: NonEmptyStr | None = None
     account: NonEmptyStr | None = None
-    extra_lines: list[NonEmptyStr] | None = None
+    extra_lines: list[NonEmptyStr] = Field(default_factory=list)
     gpus: NonEmptyStr | None = None
 
 
@@ -125,4 +126,4 @@ class JobRunnerConfigSLURM(BaseModel):
     default_slurm_config: _SlurmConfigSet
     gpu_slurm_config: _SlurmConfigSet | None = None
     batching_config: _BatchingConfigSet
-    user_local_exports: DictStrStr | None = None
+    user_local_exports: DictStrStr = Field(default_factory=dict)

fractal_server/runner/executors/slurm_common/base_slurm_runner.py

@@ -252,7 +252,7 @@ class method is useful to fix issue #2659 (which was due to
                 f"Add {self.common_script_lines} to "
                 f"{new_slurm_config.extra_lines=}."
             )
-            current_extra_lines = new_slurm_config.extra_lines or []
+            current_extra_lines = new_slurm_config.extra_lines
             new_slurm_config.extra_lines = (
                 current_extra_lines + self.common_script_lines
             )

fractal_server/runner/executors/slurm_common/get_slurm_config.py

@@ -77,7 +77,7 @@ def _get_slurm_config_internal(
     else:
         needs_gpu = False
     logger.debug(f"[get_slurm_config] {needs_gpu=}")
-    if needs_gpu:
+    if needs_gpu and shared_config.gpu_slurm_config is not None:
         for key, value in shared_config.gpu_slurm_config.model_dump(
             exclude_unset=True, exclude={"mem"}
         ).items():

fractal_server/runner/executors/slurm_common/slurm_config.py

@@ -83,10 +83,10 @@ class SlurmConfig(BaseModel):
 
     # Free-field attribute for extra lines to be added to the SLURM job
     # preamble
-    extra_lines: list[str] | None = Field(default_factory=list)
+    extra_lines: list[str] = Field(default_factory=list)
 
     # Variables that will be `export`ed in the SLURM submission script
-    user_local_exports: dict[str, str] | None = None
+    user_local_exports: dict[str, str] = Field(default_factory=dict)
 
     # Metaparameters needed to combine multiple tasks in each SLURM job
     tasks_per_job: int | None = None
@@ -152,9 +152,8 @@ def to_sbatch_preamble(
                 "SlurmConfig.sbatch_preamble requires that "
                 f"{self.parallel_tasks_per_job=} is not None."
             )
-        if self.extra_lines:
-            if len(self.extra_lines) != len(set(self.extra_lines)):
-                raise ValueError(f"{self.extra_lines=} contains repetitions")
+        if len(self.extra_lines) != len(set(self.extra_lines)):
+            raise ValueError(f"{self.extra_lines=} contains repetitions")
 
         mem_per_job_MB = self.parallel_tasks_per_job * self.mem_per_task_MB
         lines = [
@@ -187,18 +186,16 @@ def to_sbatch_preamble(
                 option = key.replace("_", "-")
                 lines.append(f"{self.prefix} --{option}={value}")
 
-        if self.extra_lines:
-            for line in self._sorted_extra_lines():
-                lines.append(line)
-
-        if self.user_local_exports:
-            if remote_export_dir is None:
-                raise ValueError(
-                    f"remote_export_dir=None but {self.user_local_exports=}"
-                )
-            for key, value in self.user_local_exports.items():
-                tmp_value = str(Path(remote_export_dir) / value)
-                lines.append(f"export {key}={tmp_value}")
+        for line in self._sorted_extra_lines():
+            lines.append(line)
+
+        if self.user_local_exports != {} and remote_export_dir is None:
+            raise ValueError(
+                f"{remote_export_dir=} but {self.user_local_exports=}"
+            )
+        for key, value in self.user_local_exports.items():
+            tmp_value = str(Path(remote_export_dir) / value)
+            lines.append(f"export {key}={tmp_value}")
 
         """
         FIXME export SRUN_CPUS_PER_TASK

tests/v2/test_08_backends/test_slurm_config.py

@@ -138,6 +138,8 @@ def test_get_slurm_config_internal_gpu_options():
             "max_num_jobs": 10,
         },
     )
+    assert shared_slurm_config.user_local_exports == {}
+    assert shared_slurm_config.default_slurm_config.extra_lines == []
 
     # In absence of `needs_gpu`, parameters in `gpu_slurm_config` are not used
     mywftask = WorkflowTaskV2Mock()