[OAR] correct check_format and lint CI tests

Author: ychiat35

submitit/__init__.py

@@ -15,9 +15,9 @@
 from .local.debug import DebugJob as DebugJob
 from .local.local import LocalExecutor as LocalExecutor
 from .local.local import LocalJob as LocalJob
-from .slurm.slurm import SlurmExecutor as SlurmExecutor
-from .slurm.slurm import SlurmJob as SlurmJob
 from .oar.oar import OarExecutor as OarExecutor
 from .oar.oar import OarJob as OarJob
+from .slurm.slurm import SlurmExecutor as SlurmExecutor
+from .slurm.slurm import SlurmJob as SlurmJob
 
 __version__ = "1.4.6"

submitit/auto/test_auto.py

@@ -10,8 +10,8 @@
 import pytest
 
 from ..local import debug
-from ..slurm import test_slurm
 from ..oar import test_oar
+from ..slurm import test_slurm
 from . import auto
 
 
@@ -39,6 +39,7 @@ def test_slurm_executor(tmp_path: Path, monkeypatch) -> None:
     with pytest.raises(NameError, match=r"debug_blublu.*\n.*local_num_threads"):
         executor.update_parameters(debug_blublu=2.0, local_num_threads=4)
 
+
 def test_oar_executor(tmp_path: Path, monkeypatch) -> None:
     monkeypatch.setattr(debug.DebugExecutor, "_valid_parameters", lambda: {"blabla"})
     with test_oar.mocked_oar():

submitit/core/plugins.py

@@ -24,13 +24,23 @@ def _get_plugins() -> Tuple[List[Type["Executor"]], List["JobEnvironment"]]:
     import pkg_resources
 
     from ..local import debug, local
-    from ..slurm import slurm
     from ..oar import oar
+    from ..slurm import slurm
 
     # TODO: use sys.modules.keys() and importlib.resources to find the files
     # We load both kind of entry points at the same time because we have to go through all module files anyway.
-    executors: List[Type["Executor"]] = [slurm.SlurmExecutor, local.LocalExecutor, debug.DebugExecutor, oar.OarExecutor]
-    job_envs = [slurm.SlurmJobEnvironment(), local.LocalJobEnvironment(), debug.DebugJobEnvironment(), oar.OarJobEnvironment()]
+    executors: List[Type["Executor"]] = [
+        slurm.SlurmExecutor,
+        local.LocalExecutor,
+        debug.DebugExecutor,
+        oar.OarExecutor,
+    ]
+    job_envs = [
+        slurm.SlurmJobEnvironment(),
+        local.LocalJobEnvironment(),
+        debug.DebugJobEnvironment(),
+        oar.OarJobEnvironment(),
+    ]
     for entry_point in pkg_resources.iter_entry_points("submitit"):
         if entry_point.name not in ("executor", "job_environment"):
             logger.warning(f"Found unknown entry point in package {entry_point.module_name}: {entry_point}")

submitit/oar/oar.py

@@ -38,7 +38,7 @@ class OarInfoWatcher(core.InfoWatcher):
     }
 
     def _make_command(self) -> tp.Optional[tp.List[str]]:
-        to_check = {x for x in self._registered - self._finished}
+        to_check = set(self._registered - self._finished)
         if not to_check:
             return None
         command = ["oarstat", "-f", "-J"]
@@ -88,7 +88,7 @@ def __init__(self, folder: tp.Union[Path, str], job_id: str, tasks: tp.Sequence[
         if len(tasks) > 1:
             raise NotImplementedError
         super().__init__(folder, job_id, tasks)
-        self._resubmitted_job = None
+        self._resubmitted_job: tp.Optional[OarJob[core.R]] = None
 
     def _interrupt(self, timeout: bool = False) -> None:
         """Sends preemption or timeout signal to the job (for testing purpose)
@@ -174,8 +174,6 @@ def _get_resubmitted_job(self) -> tp.Optional["OarJob[core.R]"]:
                         folder=self._paths.folder, job_id=resubmitted_job_id, tasks=[0]
                     )
                     return self._resubmitted_job
-                else:
-                    return None
             except Exception as e:
                 logger.get_logger().error(
                     f"Getting error with _get_resubmitted_job() by command {command}:\n"
@@ -259,7 +257,7 @@ def _internal_update_parameters(self, **kwargs: tp.Any) -> None:
         Parameters
         ----------
         See oar documentation for most parameters.
-        Most useful parameters are: core, walltime, gpu, queue.
+        Most useful parameters are: cores, walltime, gpu, queue.
 
         Below are the parameters that differ from OAR documentation:
 
@@ -326,7 +324,7 @@ def _internal_process_submissions(
         if any(isinstance(d.function, helpers.Checkpointable) for d in delayed_submissions) and any(
             not isinstance(d.function, helpers.Checkpointable) for d in delayed_submissions
         ):
-            raise Exception(
+            raise ValueError(
                 "OarExecutor does not support a job array that mixes checkpointable and non-checkpointable functions."
                 "\nPlease make groups of similar function calls in the job array."
             )
@@ -434,7 +432,7 @@ def _make_oarsub_string(
     folder: tp.Union[str, Path],
     map_count: tp.Optional[int] = None,  # used internally
     nodes: tp.Optional[int] = None,
-    core: tp.Optional[int] = None,
+    cores: tp.Optional[int] = None,
     gpu: tp.Optional[int] = None,
     walltime: tp.Optional[str] = None,
     timeout_min: tp.Optional[int] = None,
@@ -449,7 +447,7 @@ def _make_oarsub_string(
     Parameters
     ----------
     See oar documentation for most parameters.
-    Most useful parameters are: core, walltime, gpu, queue.
+    Most useful parameters are: cores, walltime, gpu, queue.
 
     Below are the parameters that differ from OAR documentation:
 
@@ -474,13 +472,13 @@ def _make_oarsub_string(
     # OAR resource hierarchy: nodes > gpu > core
     resource_hierarchy = ""
     if nodes is not None:
-        resource_hierarchy += "/nodes=%d" % nodes
+        resource_hierarchy += f"/nodes={nodes}"
     if gpu is not None:
-        resource_hierarchy += "/gpu=%d" % gpu
-    if core is not None:
-        resource_hierarchy += "/core=%d" % core
+        resource_hierarchy += f"/gpu={gpu}"
+    if cores is not None:
+        resource_hierarchy += f"/core={cores}"
     if walltime is not None:
-        walltime = "walltime=%s" % walltime
+        walltime = f"walltime={walltime}"
     resource_request = ",".join(filter(None, (resource_hierarchy, walltime)))
     if resource_request:
         parameters["l"] = resource_request

submitit/oar/test_oar.py

@@ -21,7 +21,6 @@
 from . import oar
 
 
-# pylint: disable=no-self-use
 class MockedSubprocess:
     """Helper for mocking subprocess calls"""
 
@@ -46,6 +45,7 @@ def __call__(self, command: tp.Sequence[str], **kwargs: tp.Any) -> bytes:
     def oarstat(self, _: tp.Sequence[str]) -> str:
         return "\n".join(self.job_oarstat.values())
 
+    # pylint: disable=unused-argument
     def oarsub(self, args: tp.Sequence[str]) -> str:
         """Create a "RUNNING" job."""
         job_id = str(self.job_count)
@@ -159,6 +159,7 @@ def test_oar_job_mocked(tmp_path: Path) -> None:
         assert "_USELESS_TEST_ENV_VAR_" not in os.environ, "Test context manager seems to be failing"
 
 
+# pylint: disable=too-many-locals
 @pytest.mark.parametrize("use_batch_api", (False, True))  # type: ignore
 def test_oar_job_array_mocked(use_batch_api: bool, tmp_path: Path) -> None:
     n = 5
@@ -271,6 +272,10 @@ def test_requeuing_checkpointable(tmp_path: Path, fast_forward_clock) -> None:
             submitted_pkl_13 = tmp_path / "13_submitted.pkl"
             assert not submitted_pkl_13.exists()
 
+    # The job 12 is terminated, but the resubmitted job 13 is still running
+    # The job should not be done
+    assert job.done(force_check=False) is False
+
     # This time the job as timed out,
     # but we have max_num_timeout=1, so we should requeue.
     # We are a little bit under the requested timedout, but close enough
@@ -288,6 +293,8 @@ def test_requeuing_checkpointable(tmp_path: Path, fast_forward_clock) -> None:
         utils.UncompletedJobError, match="timed-out too many times."
     ):
         sig.checkpoint_and_try_requeue(usr_sig)
+        # This time the job should be done
+        assert job.done(force_check=False) is True
 
 
 def test_requeuing_not_checkpointable(tmp_path: Path, fast_forward_clock) -> None:
@@ -376,7 +383,7 @@ def test_make_oarsub_string_gpu() -> None:
 
 
 def test_make_oarsub_string_core() -> None:
-    string = oar._make_oarsub_string(command="blublu", folder="/tmp", core=2)
+    string = oar._make_oarsub_string(command="blublu", folder="/tmp", cores=2)
     assert "-l /core=2" in string
 
 
@@ -385,13 +392,13 @@ def test_make_oarsub_string_gpu_and_nodes() -> None:
     assert "-l /nodes=1/gpu=2" in string
 
 
-def test_make_oarsub_string_core_and_nodes() -> None:
-    string = oar._make_oarsub_string(command="blublu", folder="/tmp", core=2, nodes=1)
+def test_make_oarsub_string_cores_and_nodes() -> None:
+    string = oar._make_oarsub_string(command="blublu", folder="/tmp", cores=2, nodes=1)
     assert "-l /nodes=1/core=2" in string
 
 
-def test_make_oarsub_string_core_gpu_and_nodes() -> None:
-    string = oar._make_oarsub_string(command="blublu", folder="/tmp", gpu=2, nodes=1, core=4)
+def test_make_oarsub_string_cores_gpu_and_nodes() -> None:
+    string = oar._make_oarsub_string(command="blublu", folder="/tmp", gpu=2, nodes=1, cores=4)
     assert "-l /nodes=1/gpu=2/core=4" in string