Skip to content

Commit

Permalink
jobs: fix job 'running state'
Browse files Browse the repository at this point in the history
So, the problem was: job's "running" state was never clear. So of
course, they would execute once, and never again. I did some sloppy job
at tracking down how this state was set and clear, and messed up what
was responsible for what.

Now:

* the job scheduler still set `job.is_running` before running it in a
  thread
* the `job.execute` method doesn't handle the running state anymore
* the job scheduler `_call` wrapper now uses `with job` to manage the
  job's state

This should work. This time.
  • Loading branch information
Exirel committed Aug 25, 2020
1 parent 8577094 commit 1b9869d
Showing 1 changed file with 11 additions and 31 deletions.
42 changes: 11 additions & 31 deletions sopel/tools/jobs.py
Original file line number Diff line number Diff line change
Expand Up @@ -185,12 +185,10 @@ def _run_job(self, job):
self._call(job)

def _call(self, job):
"""Wrapper for collecting errors from plugins."""
"""Wrap the job's execution to handle its state and errors."""
try:
job.execute(self.manager)
except KeyboardInterrupt:
# Do not block on KeyboardInterrupt
raise
with job:
job.execute(self.manager)
except Exception as error: # TODO: Be specific
LOGGER.error('Error while processing job: %s', error)
self.manager.on_job_error(self, job, error)
Expand Down Expand Up @@ -448,36 +446,18 @@ def next(self, current_time):
return self

def execute(self, manager):
"""Execute the job's handler and mark itself as running.
"""Execute the job's handler and return its result
:param object manager: used as argument to the job's handler
:return: the return value from the handler's execution
This method executes the job's handler, and while doing so it will
update its running state accordingly:
1. mark the job as running (set :attr:`is_running`) if not already set
2. execute the handler
3. if it wasn't already running before, update the next time the
job can run (:meth:`next`), then mark it as not running anymore
(clear :attr:`is_running`)
Then it returns the handler's return value.
This method executes the job's handler. It doesn't change its running
state, as this must be done by the caller::
.. note::
This method can be safely used with a ``with`` statement, as it
sets/clears the :attr:`is_running` event only if not previously set
by the caller.
with job: # mark as running
# before execution
job.execute(manager)
# after execution
"""
was_running = self.is_running.is_set()
try:
self.is_running.set()
result = self._handler(manager)
finally:
if not was_running:
self.next(time.time())
self.is_running.clear()

return result
return self._handler(manager)

0 comments on commit 1b9869d

Please sign in to comment.