Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Deprecate the runjobs management command #4470

Open
iaindillingham opened this issue Jul 19, 2024 · 1 comment
Open

Deprecate the runjobs management command #4470

iaindillingham opened this issue Jul 19, 2024 · 1 comment
Labels
deck-scrubbing Tech debt or other between-initiative tidy-up work

Comments

@iaindillingham
Copy link
Member

iaindillingham commented Jul 19, 2024

The runjobs management command allows us to group jobs by their schedule (daily, weekly, monthly, etc.), and make one call for the schedule rather than one call for each job. Doing so requires us to create a module for each schedule (e.g. jobserver.jobs.daily), and a module for each job (e.g. jobserver.jobs.daily.my_job) with the following structure:

from django_extensions.management.jobs import DailyJob

class Job(DailyJob):
    help = "My daily job"

    def execute(self):
        do_something()

Notice that the schedule is encoded in the module name (jobserver.jobs.daily) and the parent class (DailyJob). It's not clear which component is used by runjobs. Also notice that runjobs doesn't actually run the jobs on the schedule: it's a management command for running the jobs associated with the schedule. We use Dokku managed cron to actually run the jobs on the schedule; we configure it in app.json.

In other words, there are three locations that encode the schedule, but only one that matters. This is confusing.

The overhead of grouping jobs by their schedule might be tolerable, if there were many jobs for each schedule. However, there are not. There is one hourly job, two daily jobs, and one weekly job. All seem like they would make good Django management commands. Indeed, one is a Django management command!

It would be better to convert the jobs to management commands, and call them directly from app.json. This would allow us to deprecate the runjobs management command.

Implementation considerations

Converting the jobs to management commands wouldn't change our approach to monitoring them with Sentry Crons. Rather than wrapping Job.execute with sentry_sdk.crons.monitor, we'd wrap Command.handle.

You may argue that doing so would involve coupling the notion of a management command to the notion of a schedule. However, the runjobs management command does something similar; it couples the notion of a job to the notion of a schedule, because it doesn't actually run the jobs on the schedule.

If we are concerned about coupling, then we could create a management command for running other management commands, and wrap this management command's Command.handle with sentry_sdk.crons.monitor. app.json would look something like this:

{
  "cron": [
    {
      "command": "python manage.py runcrons hourly management_command_1 management_command_2",
      "schedule": "@hourly"
    },
    {
      "command": "python manage.py runcrons daily management_command_3",
      "schedule": "@daily"
    }
  ]
}

(Notice that app.json doesn't have entries it doesn't need: no management commands are run on weekly or monthly schedules.)

This approach may involve creating a decorator. For more information about creating decorators, see Chapter 9 of Fluent Python, Second Edition, "Decorators and Closures". (We can access this chapter using our Oxford SSOs.)

Ideally, we'd configure Sentry Crons alongside Dokku managed cron, but that doesn't seem to be possible.

@lucyb lucyb added the deck-scrubbing Tech debt or other between-initiative tidy-up work label Aug 13, 2024
@lucyb
Copy link
Contributor

lucyb commented Aug 13, 2024

This will be a good task for the next round of deck scrubbing. However, we're now starting to focus on the next initiative and won't pick it up for a while, so I'm going to move it off the board.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
deck-scrubbing Tech debt or other between-initiative tidy-up work
Projects
None yet
Development

No branches or pull requests

2 participants