jupyter-scheduler
jupyter-scheduler copied to clipboard
task runner scheduling logic concerns
For clarity, I suggest we make the following changes:
- 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 converttask.next_run_timeto UTC. - 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 theJobDefinitiontable model or in theCreateJobDefinitionrequest model.
Originally posted by @dlqqq in https://github.com/jupyter-server/jupyter-scheduler/pull/106#discussion_r990568083
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().
- Call
continueand pop the task from the queue if cache record doesn't exist- Needs to happen before the below line
- Current implementation
- Move the
if time_diff < 0: breakconditional as far up in thewhileloop as possible - Remove the
elif time_diff >= (self.poll_interval * 1000): breakblock - Rest of the
whileloop continues as normal