Skip to content

Commit 9339e12

Browse files
fix: avoid 'None' prefix in SlurmExecutor job names (#565)
SlurmExecutor.job_name_prefix defaults to None, so f-strings in alloc() and srun() produced job names like 'Noneinteractive'. Use so the prefix is omitted when unset, matching the existing handling in other methods. Adds regression tests for both alloc() and srun(). Signed-off-by: Andrew White <andrewh@cdw.com>
1 parent 5102d05 commit 9339e12

2 files changed

Lines changed: 77 additions & 2 deletions

File tree

nemo_run/core/execution/slurm.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -434,7 +434,7 @@ def info(self) -> str:
434434
return f"{self.__class__.__qualname__} on {self.tunnel.key}"
435435

436436
def alloc(self, job_name="interactive"):
437-
self.job_name = f"{self.job_name_prefix}{job_name}"
437+
self.job_name = f"{self.job_name_prefix or ''}{job_name}"
438438
args = [
439439
f"--{arg}={getattr(self, arg.replace('-', '_'))}"
440440
for arg in self.ALLOC_ARGS
@@ -457,7 +457,7 @@ def srun(
457457
arg_dict=None,
458458
**kwargs,
459459
):
460-
self.job_name = f"{self.job_name_prefix}{job_name}"
460+
self.job_name = f"{self.job_name_prefix or ''}{job_name}"
461461
_arg_dict = {
462462
arg: getattr(self, arg.replace("-", "_"))
463463
for arg in self.SRUN_ARGS
Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,75 @@
1+
# SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved.
2+
# SPDX-License-Identifier: Apache-2.0
3+
#
4+
# Licensed under the Apache License, Version 2.0 (the "License");
5+
# you may not use this file except in compliance with the License.
6+
# You may obtain a copy of the License at
7+
#
8+
# http://www.apache.org/licenses/LICENSE-2.0
9+
#
10+
# Unless required by applicable law or agreed to in writing, software
11+
# distributed under the License is distributed on an "AS IS" BASIS,
12+
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
# See the License for the specific language governing permissions and
14+
# limitations under the License.
15+
16+
from unittest.mock import MagicMock, PropertyMock, patch
17+
18+
from nemo_run.core.execution.slurm import SlurmExecutor
19+
from nemo_run.core.tunnel.client import LocalTunnel
20+
21+
22+
class TestSlurmJobNamePrefix:
23+
def test_alloc_without_prefix_uses_job_name_only(self):
24+
"""alloc() must not prepend 'None' when job_name_prefix is unset."""
25+
executor = SlurmExecutor(account="test", tunnel=LocalTunnel(job_dir="/tmp"))
26+
assert executor.job_name_prefix is None
27+
28+
mock_slurm = MagicMock()
29+
with patch.object(
30+
SlurmExecutor, "slurm", new_callable=PropertyMock, return_value=mock_slurm
31+
):
32+
executor.alloc(job_name="interactive")
33+
34+
assert executor.job_name == "interactive"
35+
36+
def test_alloc_with_prefix_prefixes_job_name(self):
37+
"""alloc() must prepend the configured prefix when set."""
38+
executor = SlurmExecutor(
39+
account="test", tunnel=LocalTunnel(job_dir="/tmp"), job_name_prefix="nemo-"
40+
)
41+
mock_slurm = MagicMock()
42+
with patch.object(
43+
SlurmExecutor, "slurm", new_callable=PropertyMock, return_value=mock_slurm
44+
):
45+
executor.alloc(job_name="interactive")
46+
47+
assert executor.job_name == "nemo-interactive"
48+
49+
def test_srun_without_prefix_uses_job_name_only(self):
50+
"""srun() must not prepend 'None' when job_name_prefix is unset."""
51+
executor = SlurmExecutor(account="test", tunnel=LocalTunnel(job_dir="/tmp"))
52+
assert executor.job_name_prefix is None
53+
54+
mock_slurm = MagicMock()
55+
with patch.object(
56+
SlurmExecutor, "slurm", new_callable=PropertyMock, return_value=mock_slurm
57+
):
58+
with patch.object(executor, "SRUN_ARGS", []):
59+
executor.srun("echo hi", job_name="interactive")
60+
61+
assert executor.job_name == "interactive"
62+
63+
def test_srun_with_prefix_prefixes_job_name(self):
64+
"""srun() must prepend the configured prefix when set."""
65+
executor = SlurmExecutor(
66+
account="test", tunnel=LocalTunnel(job_dir="/tmp"), job_name_prefix="nemo-"
67+
)
68+
mock_slurm = MagicMock()
69+
with patch.object(
70+
SlurmExecutor, "slurm", new_callable=PropertyMock, return_value=mock_slurm
71+
):
72+
with patch.object(executor, "SRUN_ARGS", []):
73+
executor.srun("echo hi", job_name="interactive")
74+
75+
assert executor.job_name == "nemo-interactive"

0 commit comments

Comments
 (0)