Skip to content

Commit 4590ff8

Browse files
committed
refactor: Launcher poll always returns a status
Previously these methods all had to account for the potential that they could be returning the uninitialized status, which was `None`. Default instead to `Queued` which means that the job is ready but not yet dispatched. In reality this `status` variable is a bit messy; it is defined on the base class as `None` and only set in `_post_finish` with the given status. It is only used by `_post_finish` itself (after being set) and is otherwise only directly used/returned by the `local`, `lsf` and `nc` launchers, each of which do not set the value. So: - It initializes as `None`. - The `local` and `nc` launcher only use it after invoking `post_finish`, so it is definitely a status then. - The `lsf` launcher only returns the status if it has a not-None status, i.e. if it already finished. So in all of these cases, `status` is only used after being set, it is just being initialized as `None` and the type propagates. Rather than perform typing casts or checks everywhere, lets just change this default value to be the `Queued` status. A larger refactor is likely needed in the future Signed-off-by: Alex Jones <[email protected]>
1 parent c75c923 commit 4590ff8

File tree

8 files changed

+11
-15
lines changed

8 files changed

+11
-15
lines changed

src/dvsim/launcher/base.py

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -119,7 +119,7 @@ def __init__(self, job_spec: "JobSpec") -> None:
119119
# _check_status() method, but eventually updated by the _post_finish()
120120
# method, in case any of the cleanup tasks fails. This value is finally
121121
# returned to the Scheduler by the poll() method.
122-
self.status = None
122+
self.status = JobStatus.QUEUED
123123

124124
# Return status of the process running the job.
125125
self.exit_code = None
@@ -259,13 +259,13 @@ def launch(self) -> None:
259259
self._do_launch()
260260

261261
@abstractmethod
262-
def poll(self) -> JobStatus | None:
262+
def poll(self) -> JobStatus:
263263
"""Poll the launched job for completion.
264264
265265
Invokes _check_status() and _post_finish() when the job completes.
266266
267267
Returns:
268-
status of the job or None
268+
status of the job
269269
270270
"""
271271

src/dvsim/launcher/fake.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,7 @@ class FakeLauncher(Launcher):
5757
def _do_launch(self) -> None:
5858
"""Do the launch."""
5959

60-
def poll(self) -> JobStatus | None:
60+
def poll(self) -> JobStatus:
6161
"""Check status of the running process."""
6262
deploy_cls = self.job_spec.job_type
6363
if deploy_cls in _DEPLOY_HANDLER:

src/dvsim/launcher/local.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -103,7 +103,7 @@ def _do_launch(self) -> None:
103103

104104
self._link_odir(JobStatus.DISPATCHED)
105105

106-
def poll(self) -> JobStatus | None:
106+
def poll(self) -> JobStatus:
107107
"""Check status of the running process.
108108
109109
This returns a job status. If DISPATCHED, the job is still running.

src/dvsim/launcher/lsf.py

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -273,15 +273,15 @@ def _do_launch(self) -> None:
273273
job.job_id = f"{job_id}[{job.index}]"
274274
job._link_odir(JobStatus.DISPATCHED)
275275

276-
def poll(self) -> JobStatus | None:
276+
def poll(self) -> JobStatus:
277277
"""Poll the status of the job.
278278
279279
Returns:
280-
status of the job or None
280+
status of the job
281281
282282
"""
283283
# It is possible we may have determined the status already.
284-
if self.status:
284+
if self.status is not JobStatus.QUEUED:
285285
return self.status
286286

287287
if not self.bsub_out_fd:

src/dvsim/launcher/nc.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -168,7 +168,7 @@ def _do_launch(self) -> None:
168168
def minutes_since_start(self):
169169
return (datetime.datetime.now() - self.start_time).total_seconds() / 60
170170

171-
def poll(self) -> JobStatus | None:
171+
def poll(self) -> JobStatus:
172172
"""Check status of the running process.
173173
174174
This returns a job status. If DISPATCHED, the job is still running.

src/dvsim/launcher/sge/launcher.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -97,7 +97,7 @@ def _do_launch(self) -> None:
9797
self._link_odir(JobStatus.DISPATCHED)
9898
f.close()
9999

100-
def poll(self) -> JobStatus | None:
100+
def poll(self) -> JobStatus:
101101
"""Check status of the running process.
102102
103103
This returns a job status. If DISPATCHED, the job is still running.

src/dvsim/launcher/slurm.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -88,7 +88,7 @@ def _do_launch(self) -> None:
8888

8989
self._link_odir(JobStatus.DISPATCHED)
9090

91-
def poll(self):
91+
def poll(self) -> JobStatus:
9292
"""Check status of the running process.
9393
9494
This returns a job status. If DISPATCHED, the job is still running.

src/dvsim/scheduler.py

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -482,10 +482,6 @@ def _poll(self, hms: str) -> bool:
482482
status = JobStatus.KILLED
483483
level = log.VERBOSE
484484

485-
if status is None:
486-
msg = "Expected a valid job status but got None instead."
487-
raise ValueError(msg)
488-
489485
if status == JobStatus.DISPATCHED:
490486
continue
491487

0 commit comments

Comments
 (0)