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.

Sign up for GitHub

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

Adding human readable descriptions of crontab schedules #622

Merged
merged 5 commits into from
Mar 2, 2023
Merged

Adding human readable descriptions of crontab schedules #622

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
a9207f4
Adding human-readable descriptions of crontab schedules
dougharris Dec 22, 2022
4a2cbc9
Adding helpful text to days of week.
dougharris Dec 22, 2022
beb471d
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Dec 22, 2022
8ac4553
Updates to make flake8 happy
dougharris Dec 22, 2022
70b424b
Merge branch 'main' into issue612
auvipy Feb 7, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 25 additions & 5 deletions django_celery_beat/admin.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ class PeriodicTaskAdmin(admin.ModelAdmin):
model = PeriodicTask
celery_app = current_app
date_hierarchy = 'start_time'
list_display = ('__str__', 'enabled', 'interval', 'start_time',
list_display = ('name', 'schedule', 'enabled', 'interval', 'start_time',
'last_run_at', 'one_off')
list_filter = ['enabled', 'one_off', 'task', 'start_time', 'last_run_at']
actions = ('enable_tasks', 'disable_tasks', 'toggle_tasks', 'run_tasks')
Expand All @@ -123,8 +123,8 @@ class PeriodicTaskAdmin(admin.ModelAdmin):
'classes': ('extrapretty', 'wide'),
}),
(_('Schedule'), {
'fields': ('interval', 'crontab', 'solar', 'clocked',
'start_time', 'last_run_at', 'one_off'),
'fields': ('interval', 'crontab', 'crontab_translation', 'solar',
'clocked', 'start_time', 'last_run_at', 'one_off'),
'classes': ('extrapretty', 'wide'),
}),
(_('Arguments'), {
Expand All @@ -138,9 +138,25 @@ class PeriodicTaskAdmin(admin.ModelAdmin):
}),
)
readonly_fields = (
'last_run_at',
'last_run_at', 'crontab_translation',
)

def crontab_translation(self, obj):
return obj.crontab.human_readable

change_form_template = 'admin/djcelery/change_periodictask_form.html'

def changeform_view(self, request, object_id=None, form_url='',
extra_context=None):
extra_context = extra_context or {}
crontabs = CrontabSchedule.objects.all()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

instead of calling all, can we reduce the number?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmmm, I'm not sure how we'd decide which to include. My goal here is to provide natural language translations for all available crontabs.

The best argument for reducing the number is possible slow response times when loading the page. But, based on other sites I've used that load a lot of data into javascript objects onto the DOM, my hunch is that there would have to be hundreds (if not thousands) of CrontabSchedule objects before it would impact performance of this page. Additionally, since this is an admin-only view, any hit to responsiveness would never be seen by end users.

Do you have thoughts on how we might choose which ones to reduce it to?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we show 20/25 per page?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's not clear to me how paginating this would work.

My goal with this change is to update the human readable description anytime the user changes the select list of defined crontabs. So, if I change from 5 * * * * to 0 10 * * 1, I want the description to change from "At 5 minutes past the hour" to "At 10:00 AM, only on Monday" to verify that the change is what I expect.

If the code only loads 20 per page, what happens if I choose a crontab that wasn't part of the 20 that were loaded?

This could be implemented as ajax/XHR so that the description is readable only on demand. Since django-celery-beat doesn't currently have any custom endpoints defined, this seems like a bigger change. Alternately, there could be a setting to disable this if a site has so many CrontabSchedules defined that loading this admin page is onerous.

Can you tell me more about your objection to having all defined on this page?

crontab_dict = {}
for crontab in crontabs:
crontab_dict[crontab.id] = crontab.human_readable
extra_context['readable_crontabs'] = crontab_dict
return super().changeform_view(request, object_id,
extra_context=extra_context)

def changelist_view(self, request, extra_context=None):
extra_context = extra_context or {}
scheduler = getattr(settings, 'CELERY_BEAT_SCHEDULER', None)
Expand Down Expand Up @@ -247,8 +263,12 @@ class ClockedScheduleAdmin(admin.ModelAdmin):
)


class CrontabScheduleAdmin(admin.ModelAdmin):
list_display = ('__str__', 'human_readable')


admin.site.register(IntervalSchedule)
admin.site.register(CrontabSchedule)
admin.site.register(CrontabSchedule, CrontabScheduleAdmin)
admin.site.register(SolarSchedule)
admin.site.register(ClockedSchedule, ClockedScheduleAdmin)
admin.site.register(PeriodicTask, PeriodicTaskAdmin)
Binary file modified django_celery_beat/locale/de/LC_MESSAGES/django.mo
View file
Open in desktop
Binary file not shown.
Loading