[Draft] [Scheduler] Added 'outdated' option to scheduler command #3771
Conversation
|
I agree, the 3rd option would probably be the best solution, seems the most robust approach. |
|
Thanks for this |
|
@maelanleborgne i'm seeing tests fail: |
|
Hi Andy, I'm sorry but I did not get the time to finish this PR, been quite
busy at work lately. The PR was still marked as draft, do you think you
could revert the commit on the target branch and reopen the PR? I'll try to
make some time for this next week.
…On Fri, Jan 5, 2024, 13:04 Andy Miller ***@***.***> wrote:
@maelanleborgne <https://github.com/maelanleborgne> i'm seeing tests fail:
Time: 00:01.018, Memory: 112.00 MB
There was 1 failure:
---------
1) JobTest: Is overdue | "hourly job created 1 hour ago and last run 30 mn ago"
Test tests/unit/Grav/Common/Scheduler/JobTest.php:testIsOverdue
Failed asserting that false matches expected true.
#1 /home/runner/work/grav/grav/tests/unit/Grav/Common/Scheduler/JobTest.php:14
#2 /home/runner/work/grav/grav/vendor/bin/codecept:119
FAILURES!
Tests: 219, Assertions: 1877, Failures: 1.
Error: Process completed with exit code 1.
—
Reply to this email directly, view it on GitHub
<#3771 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AC3PMT4AEX3JDHCBEYPF4CLYM7T5LAVCNFSM6AAAAAA7BOAOWWVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQNZYGU3DEMJUHA>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
|
Weird, it's passing now.. was in the automated tests.. i couldn't replicate locally: https://github.com/getgrav/grav/actions/runs/7421540097 Must of been a time related issue? |
|
And appologies, i missed the DRAFT bit.. worked find in my quick local testing and tests ran fine... I'll revert though. |
This reverts commit a71403f. # Conflicts: # tests/unit/Grav/Common/Scheduler/SchedulerTest.php
|
This has been reverted, but you will probably need to recreate the PR because i can't reopen it once merged. Applogies again. |
Yes I messed up the tests, will fix with the new implementation. |
Description
This PR follows #3770 and adds an
outdatedoption to the scheduler command.This allows the scheduler to run jobs that should have been processed earlier but did not (use cases : server downtime, cron not running every minute).
Usage
Advice wanted
First of all if you have another idea for the option name I'm open to discussion, I don't think
outdatedis really relevant.Now the main concern :
While working on this feature I realized the Job
creationTimeis not stored in the config, but rather set when instantiating the job. The logic I had in mind involved checking if the job had been created later than the previous scheduled execution, to prevent running a job before its first scheduled run.4bb4069#diff-d62f46dc32787f2a5ecbcca58aa4601fc942e8b293a6cfa7a9c6286475cbac02R212
I see a few options to work around this :
creationTimein thescheduler.yamlconfig. This would work for custom jobs, but the default jobs don't store their config there so I don't think it would be such a good idea.data/scheduler/status.yamlwithstate: newandlast-run: previous-scheduled-time. This way we can keep track of every change in the scheduler configuration and make sure we don't run the jobs before the next occurrence.IMO the third option would be the best to cover all cases, but I'd be happy to hear your thoughts about this :)