jupyter-server / jupyter-scheduler

Run Jupyter notebooks as jobs
https://jupyter-scheduler.readthedocs.io
BSD 3-Clause "New" or "Revised" License
196 stars 23 forks source link

task runner scheduling logic concerns #119

Open dlqqq opened 2 years ago

dlqqq commented 2 years ago
    For clarity, I suggest we make the following changes:
  1. When comparing times, I suggest we always convert them to UTC first for clarity. This logic converts the current time to the task's specified time zone, which I think is much more confusing (although equivalent). This means we can delete get_localized_timestamp() and instead just convert task.next_run_time to UTC.
  2. It's very confusing that the default timezone (UTC) is set by TaskRunner.compute_time_diff() in a ternary statement. When setting default values for records, we should set them as "high up" as possible for visibility. I suggest making job definition timezones non-optional and enforcing the default value in either the JobDefinition table model or in the CreateJobDefinition request model.

_Originally posted by @dlqqq in https://github.com/jupyter-server/jupyter-scheduler/pull/106#discussion_r990568083_

dlqqq commented 1 year ago

Thoughts I had while reviewing #253:

The bug documented in #253 partly originated from the fact that we're "overly eager" about removing tasks from the queue even when we don't need to. I believe the workaround I suggested in #253 could be removed if we change the body of the while loop in process_queue().