Skip to content

CronJob.batched_run schedules only the first task and never returns #1905

Description

@ayaangazali

Found while fixing #1904 in the same class. CronJob.batched_run(tasks) schedules the first task and then blocks forever, so the rest of the list is never scheduled and the method never returns.

def run(self, task: str, **kwargs):
    try:
        job = self._run(task, **kwargs)
        while True:                  # <- blocks here for the life of the process
            time.sleep(1)
        return job                   # <- unreachable
    ...

def batched_run(self, tasks: List[str], **kwargs):
    outputs = []
    for task in tasks:               # <- iteration 2 is never reached
        output = self.run(task, **kwargs)
        outputs.append(output)
    return outputs

Verified with a recording callable and a 1-second interval:

job = CronJob(agent=rec, interval="1second")
threading.Thread(target=lambda: job.batched_run(["task-A", "task-B", "task-C"])).start()
time.sleep(5)
tasks actually scheduled: ['task-A']
batched_run never returned: True

Only task-A ever runs. task-B and task-C are silently dropped, and the return outputs is dead. The only way out of run() is KeyboardInterrupt, which calls self.stop() and returns None — so if a user does Ctrl-C, batched_run then proceeds to schedule task-B on a scheduler that has just been stopped.

Also worth noting: run() returns job after the infinite loop, so outputs would be a list of None even if the loop were escapable — _run() returns nothing.

The reason this needs a decision rather than a patch is what batched_run is supposed to mean:

  1. Schedule all, then block oncefor task in tasks: self._run(task, **kwargs) followed by a single blocking wait. This matches "run these tasks on a schedule" and is what I would expect the intent to be. It changes run()'s structure, since the blocking loop has to move out of it.
  2. Run each task to completion in turn — only coherent for a one-shot execution, which is not what a cron job does.
  3. Deprecate it — if the intended usage is one task per CronJob, batched_run is a footgun and deleting it is smaller than fixing it.

I lean toward (1), with the blocking wait extracted into something like _block_forever() that both run and batched_run call after scheduling. Happy to send that as a PR if you agree on the semantics.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions