From f91402929ac337d54fd310fa2b9ff5e78dd99cfa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Petr=20H=C3=A1la?= Date: Sat, 9 Sep 2023 21:02:15 +0200 Subject: [PATCH] Migrate to using django-rq for scheduled calls (#63) - CRON is no longer needed - Improvements to Request list page --- pdf/generate.py | 51 +++++++++++- pdf/management/commands/generate_pdf.py | 2 +- ...remove_pdfrequest_created_date_and_more.py | 27 +++++++ pdf/models/request.py | 6 +- pdf/templates/pdf/requests/list.html | 41 ++++++---- pdf/templatetags/types.py | 2 +- pdf/utils.py | 77 ++++++++----------- pdf/views.py | 3 +- 8 files changed, 141 insertions(+), 68 deletions(-) create mode 100644 pdf/migrations/0020_remove_pdfrequest_created_date_and_more.py diff --git a/pdf/generate.py b/pdf/generate.py index bddf610..4a3df26 100644 --- a/pdf/generate.py +++ b/pdf/generate.py @@ -2,22 +2,25 @@ import locale import logging import os +import re import tempfile +from datetime import datetime from math import ceil +from time import time import weasyprint +from rq import Retry from weasyprint.logger import PROGRESS_LOGGER from django.conf import settings from django.core.files import File from django.template.loader import render_to_string from django.urls import reverse from django.utils import translation -from django_rq import job +from django_rq import job, get_queue from django_weasyprint.utils import django_url_fetcher from pdf.locales import changed_locale, lang_to_locale from pdf.models.request import PDFRequest, Status -from pdf.utils import Timer, ProgressFilter logger = logging.getLogger(__name__) logger.setLevel(logging.INFO) @@ -88,3 +91,47 @@ def generate_pdf(request: PDFRequest): def generate_pdf_job(request: PDFRequest): """Generates PDF from request in the background""" generate_pdf(request) + + +def schedule_generation(request: PDFRequest, schedule_time: datetime): + """Schedules generation of a request at a specific time""" + queue = get_queue('default') + created_job = queue.enqueue_at( + schedule_time, + generate_pdf, + request, + retry = Retry(max=5, interval=120) + ) + logger.info("Schedule PDF generation of request %s at %s", request.id, schedule_time) + return created_job + + +class ProgressFilter(logging.Filter): + """Filters Weasyprint progress messages, highly dependent on implementation!""" + STEP_NUMBER = re.compile(r"(?<=Step\s)\d") + + def __init__(self, request): + super().__init__() + self.request = request + + def filter(self, record): + step = self.STEP_NUMBER.search(record.getMessage()).group(0) + if self.request.progress != step: + self.request.progress = step + self.request.save() + return True + + +class Timer: + """Context manager for measuring time""" + def __init__(self): + self.duration = 0 + self.start = 0 + self.end = 0 + + def __enter__(self): + self.start = time() + + def __exit__(self, exit_type, value, traceback): + self.end = time() + self.duration = self.end - self.start diff --git a/pdf/management/commands/generate_pdf.py b/pdf/management/commands/generate_pdf.py index 5ec6a49..387c5a0 100644 --- a/pdf/management/commands/generate_pdf.py +++ b/pdf/management/commands/generate_pdf.py @@ -42,7 +42,7 @@ def handle(self, *args, **options): if all_requests: objects = [generate_new_pdf_request(category) for category in Category.objects.filter(generate_pdf=True)] else: - objects = PDFRequest.objects.filter(status=Status.QUEUED) + objects = PDFRequest.objects.filter(status__in={Status.QUEUED, Status.SCHEDULED}) if requests: objects = objects[:requests] diff --git a/pdf/migrations/0020_remove_pdfrequest_created_date_and_more.py b/pdf/migrations/0020_remove_pdfrequest_created_date_and_more.py new file mode 100644 index 0000000..e45ad9c --- /dev/null +++ b/pdf/migrations/0020_remove_pdfrequest_created_date_and_more.py @@ -0,0 +1,27 @@ +# Generated by Django 4.1.10 on 2023-09-09 18:53 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('pdf', '0019_pdfrequest_public'), + ] + + operations = [ + migrations.RemoveField( + model_name='pdfrequest', + name='created_date', + ), + migrations.AddField( + model_name='pdfrequest', + name='scheduled_at', + field=models.DateTimeField(null=True), + ), + migrations.AlterField( + model_name='pdfrequest', + name='status', + field=models.CharField(choices=[('QU', 'Queued'), ('SC', 'Scheduled'), ('PR', 'In progress'), ('DO', 'Done'), ('FA', 'Failed')], default='QU', max_length=2), + ), + ] diff --git a/pdf/models/request.py b/pdf/models/request.py index a7f116f..7674397 100644 --- a/pdf/models/request.py +++ b/pdf/models/request.py @@ -3,7 +3,7 @@ from django.core.validators import MinValueValidator from django.db.models import Model, DateField, DateTimeField, IntegerField, FileField, CharField, TextChoices, \ - ManyToManyField, ForeignKey, CASCADE, SET_NULL, CheckConstraint, Q, PositiveIntegerField + ManyToManyField, ForeignKey, CASCADE, SET_NULL, CheckConstraint, Q, PositiveIntegerField, TextField from django.utils.translation import gettext_lazy as _ from backend.models import Song @@ -23,6 +23,7 @@ class RequestType(TextChoices): class Status(TextChoices): """Status of PDF Request""" QUEUED = "QU", _('Queued') + SCHEDULED = "SC", _('Scheduled') IN_PROGRESS = "PR", _('In progress') DONE = "DO", _("Done") FAILED = "FA", _("Failed") @@ -30,7 +31,7 @@ class Status(TextChoices): class PDFRequest(PDFOptions): """Request for PDF generation""" - created_date = DateField(auto_now_add=True, editable=False) +# created_date = DateField(auto_now_add=True, editable=False) update_date = DateTimeField(auto_now=True) type = CharField( max_length=2, @@ -47,6 +48,7 @@ class PDFRequest(PDFOptions): file = FileField(null=True, storage=fs) songs = ManyToManyField(Song, through="PDFSong") category = ForeignKey(Category, null=True, on_delete=SET_NULL) + scheduled_at = DateTimeField(null=True) def get_songs(self) -> List[Song]: """Returns all songs for request""" diff --git a/pdf/templates/pdf/requests/list.html b/pdf/templates/pdf/requests/list.html index de9f117..f9070e0 100644 --- a/pdf/templates/pdf/requests/list.html +++ b/pdf/templates/pdf/requests/list.html @@ -6,12 +6,22 @@ {% block header %} {% trans "PDF Requests" %} {% endblock %} {% block extra_head %} - - + + {% endblock %} {% block framed_body %} -
+
+ + + +
+ +
{% trans "Search" %}
@@ -22,8 +32,6 @@ {% trans "Title" %} - {# {% trans "Filename" %}#} - {% trans "Date Created" %} {% trans "Last updated" %} {% trans "Type" %} {% trans "Status" %} @@ -38,17 +46,9 @@ {% for request in requests %} {{ request.title }} - {# #} - {# {% if request.filename %}#} - {# {{ request.filename }}#} - {# {% else %}#} - {# {% trans "Automatic" %}#} - {# {% endif %}#} - {# #} - {{ request.created_date}} {{ request.update_date }} {{ request.get_type_display }} - {{ request.get_status_display }} {% if request.status == "PR" %}{{ request.progress }}/7 {% endif %} + {{ request.get_status_display }} {% if request.status == "PR" %}{{ request.progress }}/7 {% endif %} {% if request.status == "SC" %} {{ request.scheduled_at }} {% endif %} {% if request.file %} {{ request.file|filename }} @@ -84,18 +84,27 @@
{% endblock %} \ No newline at end of file diff --git a/pdf/templatetags/types.py b/pdf/templatetags/types.py index 0865c90..573242b 100644 --- a/pdf/templatetags/types.py +++ b/pdf/templatetags/types.py @@ -11,7 +11,7 @@ @register.filter(is_safe=True) def get_status_color(status): """Converts status to color""" - if status == str(Status.QUEUED): + if status == str(Status.QUEUED) or status == str(Status.SCHEDULED): return "yellow" if status == str(Status.IN_PROGRESS): return "blue" diff --git a/pdf/utils.py b/pdf/utils.py index 91a9e20..ab54226 100644 --- a/pdf/utils.py +++ b/pdf/utils.py @@ -1,35 +1,18 @@ """Utility functions""" -import logging -import re -from time import time +from datetime import timedelta +from django.utils import timezone -from django.conf import settings from django.db import transaction from django.utils import translation from django.utils.translation import gettext +from pdf.generate import schedule_generation from pdf.models.request import PDFRequest, RequestType, Status, PDFSong -class ProgressFilter(logging.Filter): - """Filters Weasyprint progress messages, highly dependent on implementation!""" - STEP_NUMBER = re.compile(r"(?<=Step\s)\d") - - def __init__(self, request): - super().__init__() - self.request = request - - def filter(self, record): - step = self.STEP_NUMBER.search(record.getMessage()).group(0) - if self.request.progress != step: - self.request.progress = step - self.request.save() - return True - - def request_pdf_regeneration(category, update: bool = False): """Requests automatic PDF regeneration if none is pending""" - objects = PDFRequest.objects.filter(type=RequestType.EVENT, status=Status.QUEUED, category=category) + objects = PDFRequest.objects.filter(type=RequestType.EVENT, status=Status.SCHEDULED, category=category) if not objects.exists(): generate_new_pdf_request(category) elif not update: @@ -37,7 +20,7 @@ def request_pdf_regeneration(category, update: bool = False): def regenerate_pdf_request(request, category): - """Regenerates the PDF request with newest info""" + """Regenerates the PDF request with the newest info""" with transaction.atomic(): PDFSong.objects.filter(request=request).delete() PDFSong.objects.bulk_create([ @@ -50,10 +33,12 @@ def regenerate_pdf_request(request, category): def generate_new_pdf_request(category): - """Returns PDFRequest for basic""" + """Returns PDFRequest for a category""" with transaction.atomic(): + scheduled_times = PDFRequest.objects.filter(status=Status.SCHEDULED, type=RequestType.EVENT).values_list( + 'scheduled_at', flat=True) request = PDFRequest(type=RequestType.EVENT, - status=Status.QUEUED, + status=Status.SCHEDULED, category=category) request.copy_options(category) request.filename = request.filename or get_filename(category) @@ -64,32 +49,34 @@ def generate_new_pdf_request(category): song_number=song_number + 1) for song_number, song in enumerate(category.song_set.filter(archived=False).all()) ]) + + time = generate_unique_time(scheduled_times, timedelta(minutes=30)) + + schedule_generation(request, time) + request.scheduled_at = time + request.save() + return request +def generate_unique_time(times, delta: timedelta): + """Generate unique time, so jobs will not clash on scheduler""" + time = timezone.now() + delta + for _ in range(len(times) + 1): + + valid = True + for existing_time in times: + if not existing_time: + continue + if time - existing_time < delta: + valid = False + + if valid: + return time + time = time + delta + raise AttributeError("Unable to assign unique generation time") def get_filename(category): """Returns filename for category based on its locale""" with translation.override(category.locale): text = gettext('songbook') return f"{text}-{category.name}" - - -def get_name(category): - """Returns filename for category based on its locale""" - with translation.override(category.locale): - return gettext(settings.SITE_NAME) - - -class Timer: - """Context manager for measuring time""" - def __init__(self): - self.duration = 0 - self.start = 0 - self.end = 0 - - def __enter__(self): - self.start = time() - - def __exit__(self, exit_type, value, traceback): - self.end = time() - self.duration = self.end - self.start diff --git a/pdf/views.py b/pdf/views.py index cf33e00..0a8939a 100644 --- a/pdf/views.py +++ b/pdf/views.py @@ -42,8 +42,9 @@ def get(self, request, pk): return redirect("pdf:list") obj.status = Status.QUEUED obj.save() + generate_pdf_job.delay(obj) - messages.success(request, _("Request %(id)s was marked for regeneration") % {"id": obj.id}) + messages.success(request, _("Request %(id)s was scheduled for regeneration") % {"id": obj.id}) return redirect("pdf:list")