Closed jonasraoni closed 1 month ago
Ah, while you're working on it, check if you need to address this comment: https://github.com/pkp/pkp-lib/issues/9979#issuecomment-2189076176
@asmecher it's ready for review. Please see the PRs at https://github.com/pkp/pkp-lib/issues/9678#issue-2110219392 .
@jonasraoni, could you have a look at this one?
@jonasraoni can you take another look at the PRs ? I have addressed most of your suggestions and if all ok, I will port the changes to OMP and OPS main . Also I have listed few plugins that need modification for this and if I have missed any, please add those to list . All details at https://github.com/pkp/pkp-lib/issues/9678#issue-2110219392
@jonasraoni can you check the plugin PRs at https://github.com/pkp/pkp-lib/issues/9678#issue-2110219392 specially for the pln . If all ok, I plan to update the submodules for OJS
and OMP
(already removed the Acron
submodule for OPS
and it's working) and should be good to merge .
@touhidurabir I've approved the other PRs! FYI I didn't test the updates locally, but there should be enough time to catch unexpected bugs until the next major release.
There are just two not resolved comments, as they are generic, I'll tag @asmecher to get a second opinion (it could be also discussed on the call/voted to be more democratic 😁).
Thanks, @jonasraoni and @touhidurabir, I've commented on both.
@jonasraoni I have merged the OJS
, OMP
and OPS
PRs but do not have the write access to plugins , details at https://github.com/pkp/pkp-lib/issues/9678#issue-2110219392 . Can you please merge the plugins PRs ?
Me neither, summoning @asmecher / @bozana to merge the remaining PRs.
ps: As we've got write access to the pkp-lib/apps, I think we should have write-access to the other repos, as they are less risky from a security perspective.
I've merged the 3 remaining PRs, thanks! Is this ready to be closed?
I've merged the 3 remaining PRs, thanks! Is this ready to be closed?
@asmecher it now good to be closed . But there was no need to merge the PR https://github.com/pkp/acron/pull/10 for acron
as thats obsolete anyway for main
. That was just basically comment out the Hooks in register so that those does not run. However it's only for main branch so i don't see any problem.
Describe the feature The task scheduler syntax isn't very friendly and has caused issues in the past.
For a next big release, it's better to discard the code and move to the Laravel scheduler, which supports better configurations and the usage of a CRON syntax.
Extra reasons If a plugin wants to run scheduled tasks, it needs to provide a
scheduledTasks.xml
file, and also attach itself to theAcron
plugin. After installing such plugin, the administrator of a journal, which is using CRON instead of the Acron plugin, will also need to add a new entry atCRON
to execute the plugin'sscheduledTasks.xml
.This is a follow-up of https://github.com/pkp/pkp-lib/issues/8921 Also considering https://github.com/pkp/pkp-lib/issues/7940
Plugins to update
PRs
Implementation Details Changes are as following :-
PKP\scheduledTask\PKPScheduler
and only app specific ones inAPP\scheduler\Scheduler
.tools/runScheduledTasks.php
has been removed and we now have few different command to run schedules as :-Better to run
php lib/pkp/tools/scheduler.php usage
to see all available options.Acron plugin
has been removed and the functionality to run schedule task on web based request has been moved to core . To enable the web based schedule task running , need to add the following inconfig.inc.php
file :-[schedule]
; Whether or not to turn on the built-in schedule task runner ; ; When enabled, schedule tasks will be processed at the end of each web ; request to the application. ; ; Use of the built-in schedule task runner is highly discouraged for high-volume ; sites. Use your operational system's task scheduler instead, and configure ; it to run the task scheduler every minute. ; Sample for the nix crontab ; php lib/pkp/tools/scheduler.php run >> /dev/null 2>&1 ; ; See:
task_runner = On
; How often should the built-in schedule task runner run scheduled tasks at the ; end of web request life cycle (value defined in seconds). ; ; This configuration will only have effect for the build-it task runner, it doesn't apply ; to the system crontab configuration. ; ; The default value is set to 60 seconds, a value smaller than that might affect the ; application performance negatively. task_runner_interval = 60
; When enabled, an email with the scheduled task result will be sent only when an error ; has occurred. Otherwise, all tasks will generate a notification. scheduled_tasks_report_error_only = On