Bug fixes and improvements #34
Conversation
(cherry picked from commit 83cc432)
Rejects when a job's interval is lower than a queue's interval. Rejects when a job's interval is not a multiple of a queue's interval. This seemed to be the most restrictive approach from which things can be loosened up. Added a settings parameter DJANGO_RQ_SCHEDULER_INTERVAL that can be used to change the schedular interval. Default is 60 matching DJANGO_RQ's default. Renamed migration adding seconds to be more descriptive.
De-duped code and renamed QueueMixin, which had grown to be more than just that. Fixed bug where user without delete permission would get error due to `del actions['delete_selected']` not existing in actions. Delete, bulk enable, and bulk disable all give messages of their success status. Made clear cron string times are in UTC.
With RQ geting a version 1.0 (from 0.13) several other packages had releases w. backwards incompatible changes (Django-RQ's v2.0 for example). This update enforces these changes and bumps rq-rcheduler due to tests failing w. lower versions. Also fixes an issue in this package causing a problem due to changes in redis v3.
Also changes Validation errors to better fit what's suggested in documentation.
Viewable to users with change permission. Pulls timeout and result_ttl from job if present. Does NOT decrement repeat count on repeatable job 🤔.
…rovements # Conflicts: # scheduler/models.py
|
@tom-price Thanks Tom, I'm going to look this over today |
| from model_utils import Choices | ||
| from model_utils.models import TimeStampedModel | ||
|
|
||
| RQ_SCHEDULER_INTERVAL = getattr(settings, "DJANGO_RQ_SCHEDULER_INTERVAL", 60) | ||
|
|
There was a problem hiding this comment.
This is currently undocumented in the README. I've not done enough testing on the ramifications of changing this from the default interval of 60 set in Django RQ's get_scheduler function.
I have written checks that prevent saving jobs where the given job_interval either shorter than this interval or not a multiple of it (seemed the safest / most restrictive approach). I felt this was necessary as part of adding seconds as an interval_unit.
| @@ -168,20 +207,36 @@ def interval_seconds(self): | |||
| } | |||
| return timedelta(**kwargs).total_seconds() | |||
|
|
|||
| def _prevent_duplicate_runs(self): | |||
There was a problem hiding this comment.
I'm pretty sure this resolves issue #30
It sounds like the problem described is caused by there being remaining repeats with a scheduled time in the past. In my experience when this happens rq-scheduler starts the job right away essentially trying to catch up. The code below essentially steps forwards in time until there are no repeats left or it arrives at some new scheduled_time in the future that is the original scheduled_time + interval*n where n is the number of repeats it took to get there and subtracted from the repeats remaining.
Seconds interval units
merge upstream
…into bug_fixes_and_improvements # Conflicts: # setup.py
…3.0, bugfix * Bugfix: rq scheduler's enqueue_at takes in a `job_result_ttl` not `result_ttl` kwarg
|
I've had to remove FakeRedis as an update broke its setup and I cannot figure it out. |
OK, I'm re-implementing whats in PR #24 in two parts and this is the first step with the args+kwargs implementation built atop it to addresses issue #1. Here's an overview of the changes:
models.pyto 100%secondsas an available interval unit following my comments. Driven by Issue Addsecondsas a option RepeatableJob.UNITS #28