Skip to content

Commit ac2c29a

Browse files
fix: log exceptions instead of silently swallowing them in Job/JobGroup.status (#569)
Job.status and JobGroup.status used with a , which swallowed status-check failures silently and made debugging hard. Preserve the existing fallback behavior (return last known state / UNKNOWN) but log the exception at ERROR level so operators can see when the runner is failing. Updates existing exception tests to assert the error is logged. Signed-off-by: Andrew White <andrewh@cdw.com>
1 parent 9ff1627 commit ac2c29a

2 files changed

Lines changed: 22 additions & 12 deletions

File tree

nemo_run/run/job.py

Lines changed: 12 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
# See the License for the specific language governing permissions and
1414
# limitations under the License.
1515

16+
import logging
1617
import sys
1718
import traceback
1819
from dataclasses import dataclass, field
@@ -28,6 +29,7 @@
2829
from nemo_run.core.execution.slurm import SlurmExecutor
2930
from nemo_run.core.frontend.console.api import CONSOLE
3031
from nemo_run.core.serialization.zlib_json import ZlibJSONSerializer
32+
3133
from nemo_run.run.logs import get_logs
3234
from nemo_run.run.plugin import ExperimentPlugin
3335
from nemo_run.run.task import direct_run_fn
@@ -36,6 +38,8 @@
3638
from nemo_run.run.torchx_backend.runner import Runner
3739
from nemo_run.run.torchx_backend.schedulers.api import get_executor_str
3840

41+
logger = logging.getLogger(__name__)
42+
3943

4044
@dataclass
4145
class Job(ConfigurableMixin):
@@ -98,9 +102,9 @@ def status(self, runner: Runner) -> AppState:
98102
status = runner.status(self.handle)
99103
state = status.state if status else None
100104
except Exception:
101-
...
102-
finally:
103-
return state or self.state
105+
logger.exception("Failed to get status for job %s", self.handle)
106+
state = None
107+
return state or self.state
104108

105109
def logs(self, runner: Runner, regex: str | None = None):
106110
get_logs(
@@ -316,11 +320,11 @@ def status(self, runner: Runner) -> AppState:
316320
status = runner.status(handle)
317321
state = status.state if status else None
318322
except Exception:
319-
...
320-
finally:
321-
if not state:
322-
state = AppState.UNKNOWN
323-
new_states.append(state)
323+
logger.exception("Failed to get status for job handle %s", handle)
324+
state = None
325+
if not state:
326+
state = AppState.UNKNOWN
327+
new_states.append(state)
324328

325329
self.states = new_states
326330
return self.state

test/run/test_job.py

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -102,7 +102,7 @@ def test_job_status_launched(simple_task, docker_executor, mock_runner):
102102
mock_runner.status.assert_called_once_with("test-handle")
103103

104104

105-
def test_job_status_exception(simple_task, docker_executor, mock_runner):
105+
def test_job_status_exception(simple_task, docker_executor, mock_runner, caplog):
106106
job = Job(
107107
id="test-job",
108108
task=simple_task,
@@ -113,7 +113,10 @@ def test_job_status_exception(simple_task, docker_executor, mock_runner):
113113
)
114114

115115
mock_runner.status.side_effect = Exception("Test exception")
116-
assert job.status(mock_runner) == AppState.RUNNING
116+
with caplog.at_level("ERROR", logger="nemo_run.run.job"):
117+
assert job.status(mock_runner) == AppState.RUNNING
118+
assert "Failed to get status for job test-handle" in caplog.text
119+
assert "Test exception" in caplog.text
117120

118121

119122
def test_job_logs(simple_task, docker_executor, mock_runner):
@@ -437,7 +440,7 @@ def test_job_group_status_launched(simple_task, docker_executor, mock_runner):
437440
mock_runner.status.assert_called_once_with("handle1")
438441

439442

440-
def test_job_group_status_exception(simple_task, docker_executor, mock_runner):
443+
def test_job_group_status_exception(simple_task, docker_executor, mock_runner, caplog):
441444
job_group = JobGroup(
442445
id="test-group",
443446
tasks=[simple_task, simple_task],
@@ -448,9 +451,12 @@ def test_job_group_status_exception(simple_task, docker_executor, mock_runner):
448451
)
449452

450453
mock_runner.status.side_effect = Exception("Test exception")
451-
status = job_group.status(mock_runner)
454+
with caplog.at_level("ERROR", logger="nemo_run.run.job"):
455+
status = job_group.status(mock_runner)
452456
assert status == AppState.UNKNOWN
453457
assert job_group.states == [AppState.UNKNOWN]
458+
assert "Failed to get status for job handle handle1" in caplog.text
459+
assert "Test exception" in caplog.text
454460

455461

456462
def test_job_group_logs(simple_task, docker_executor, mock_runner):

0 commit comments

Comments
 (0)