Skip to content

Commit ca285bb

Browse files
fix: poll job.status() once per job in Experiment.status() (#582)
Experiment.status() called job.status(runner=...) twice while building one status row: once for the display text and again for the returned dictionary. A backend failure therefore produced two identical ERROR tracebacks per status request and wasted a redundant status poll. Store the status in a local variable and reuse it for both the display string and the dict. Add a regression test that asserts job.status() is called exactly once for both return_dict=True and the default print path. Signed-off-by: Andrew White <andrewh@cdw.com> Co-authored-by: oliver könig <okoenig@nvidia.com>
1 parent 514d076 commit ca285bb

2 files changed

Lines changed: 27 additions & 4 deletions

File tree

nemo_run/run/experiment.py

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -908,10 +908,9 @@ def _get_job_info_and_dict(
908908
idx: int, job: Job | JobGroup
909909
) -> tuple[list[str], dict[str, str]]:
910910
job_info = []
911+
job_status = job.status(runner=self._runner)
911912
job_info.append(f"[bold green]Task {idx}[/bold green]: [bold orange1]{job.id}")
912-
job_info.append(
913-
f"- [bold green]Status[/bold green]: {str(job.status(runner=self._runner))}"
914-
)
913+
job_info.append(f"- [bold green]Status[/bold green]: {str(job_status)}")
915914
job_info.append(f"- [bold green]Executor[/bold green]: {job.executor.info()}")
916915

917916
try:
@@ -927,7 +926,7 @@ def _get_job_info_and_dict(
927926
]
928927
job_dict = {
929928
"name": job.id,
930-
"status": job.status(runner=self._runner),
929+
"status": job_status,
931930
"executor": job.executor.info(),
932931
"job_id": app_id,
933932
"handle": job.handle,

test/run/test_experiment.py

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -684,6 +684,30 @@ def test_experiment_status(mock_get_runner, temp_dir):
684684
mock_print.assert_called()
685685

686686

687+
@patch("nemo_run.run.experiment.get_runner")
688+
def test_experiment_status_polls_job_once(mock_get_runner, temp_dir):
689+
"""Experiment.status() should call job.status() once per job.
690+
691+
Previously the display text and returned dict each called job.status(),
692+
producing duplicate backend requests (and duplicate error logs on failure).
693+
"""
694+
mock_runner = MagicMock()
695+
mock_get_runner.return_value = mock_runner
696+
697+
with Experiment("test-exp") as exp:
698+
task = run.Partial(dummy_function, x=1, y=2)
699+
exp.add(task, name="test-job")
700+
exp.jobs[0].status = MagicMock(return_value=AppState.SUCCEEDED)
701+
702+
exp.status(return_dict=True)
703+
exp.jobs[0].status.assert_called_once_with(runner=mock_runner)
704+
705+
exp.jobs[0].status.reset_mock()
706+
with patch.object(exp.console, "print"):
707+
exp.status()
708+
exp.jobs[0].status.assert_called_once_with(runner=mock_runner)
709+
710+
687711
@patch("nemo_run.run.experiment.get_runner")
688712
def test_experiment_cancel(mock_get_runner, temp_dir):
689713
"""Test cancelling an experiment job."""

0 commit comments

Comments
 (0)