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.

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

Replace task scheduler by Laravel scheduler #9678

Closed
5 of 6 tasks
jonasraoni opened this issue Jan 31, 2024 · 11 comments
Closed
5 of 6 tasks

Replace task scheduler by Laravel scheduler #9678

jonasraoni opened this issue Jan 31, 2024 · 11 comments
Assignees
Labels
Enhancement:3:Major A new feature or improvement that will take a month or more to complete. Housekeeping:1:Todo Any dependency management or refactor that would be nice to have some day.
Milestone

Comments

@jonasraoni
Copy link
Contributor

jonasraoni commented Jan 31, 2024

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 the Acron 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 at CRON to execute the plugin's scheduledTasks.xml.

  • This is a bit counter-intuitive, the Acron shouldn't be any simpler than using CRON after both are setup.
  • The "registration" should be unified, the executor just needs to have access to the list of tasks and their execution periods.

This is a follow-up of #8921
Also considering #7940

Plugins to update

PRs

Implementation Details
Changes are as following :-

  1. the XML based task scheduler is removed and moved to use the laravel like task scheduling approach, see at https://laravel.com/docs/11.x/scheduling . To register a schedule task that shared by all app , register it in PKP\scheduledTask\PKPScheduler and only app specific ones in APP\scheduler\Scheduler.
  2. The old schedule tool tools/runScheduledTasks.php has been removed and we now have few different command to run schedules as :-
php lib/pkp/tools/scheduler.php run # Run any pending schedule task 
php lib/pkp/tools/scheduler.php list # List all schedule tasks registered
php lib/pkp/tools/scheduler.php work # Run the schedule tasks as daemon, useful for local dev rather than to set up a crontab
php lib/pkp/tools/scheduler.php test # Allow the run specific schedule task from the list, useful for devs to test during development

Better to run php lib/pkp/tools/scheduler.php usage to see all available options.
3. The 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 in config.inc.php file :-

;;;;;;;;;;;;;;;;;;;;;;;;;;
; Schedule Task Settings ;
;;;;;;;;;;;;;;;;;;;;;;;;;;

[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: <link-to-documentation>
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
  1. the previously existed config option scheduled_tasks_report_error_only has been moved from general section to newly added section schedule in config.inc.php
  2. config option scheduled_task from section general has been removed in favour of Use or remove scheduled_tasks flag in config.inc.php #7940 .
  3. Plugins now also can not register schedule tasks as XML but to use the laravel based scheduler convention . To plugins have own scheduler, it need to implement the PKP\plugins\interfaces\HasTaskScheduler and register the task, for example :
/**
     * @copydoc \PKP\plugins\interfaces\HasTaskScheduler::registerSchedules()
     */
    public function registerSchedules(PKPScheduler $scheduler): void
    {
        $scheduler
            ->addSchedule(new DOAJInfoSender())
            ->everyMinute()
            ->name(DOAJInfoSender::class)
            ->withoutOverlapping();
    }
@jonasraoni
Copy link
Contributor Author

Ah, while you're working on it, check if you need to address this comment: #9979 (comment)

touhidurabir added a commit to touhidurabir/pkp-lib that referenced this issue Jun 27, 2024
touhidurabir added a commit to touhidurabir/ojs that referenced this issue Jun 27, 2024
touhidurabir added a commit to touhidurabir/pkp-lib that referenced this issue Jun 28, 2024
touhidurabir added a commit to touhidurabir/ojs that referenced this issue Jun 28, 2024
@touhidurabir touhidurabir added Enhancement:3:Major A new feature or improvement that will take a month or more to complete. Housekeeping:1:Todo Any dependency management or refactor that would be nice to have some day. and removed Enhancement:2:Moderate A new feature or improvement that can be implemented in less than 4 weeks. labels Jul 1, 2024
touhidurabir added a commit to touhidurabir/pkp-lib that referenced this issue Jul 1, 2024
touhidurabir added a commit to touhidurabir/pkp-lib that referenced this issue Jul 1, 2024
touhidurabir added a commit to touhidurabir/pkp-lib that referenced this issue Jul 1, 2024
touhidurabir added a commit to touhidurabir/pkp-lib that referenced this issue Jul 1, 2024
touhidurabir added a commit to touhidurabir/ojs that referenced this issue Jul 1, 2024
touhidurabir added a commit to touhidurabir/ojs that referenced this issue Jul 1, 2024
touhidurabir added a commit to touhidurabir/pkp-lib that referenced this issue Jul 1, 2024
touhidurabir added a commit to touhidurabir/ojs that referenced this issue Jul 1, 2024
@touhidurabir
Copy link
Member

@jonasraoni can you check the plugin PRs at #9678 (comment) 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 .

jonasraoni added a commit to touhidurabir/pkp-pln that referenced this issue Jul 30, 2024
@jonasraoni
Copy link
Contributor Author

@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 😁).

@asmecher
Copy link
Member

Thanks, @jonasraoni and @touhidurabir, I've commented on both.

touhidurabir added a commit to touhidurabir/pkp-lib that referenced this issue Jul 31, 2024
touhidurabir added a commit to touhidurabir/ojs that referenced this issue Jul 31, 2024
touhidurabir added a commit to touhidurabir/ojs that referenced this issue Jul 31, 2024
touhidurabir added a commit to touhidurabir/omp that referenced this issue Jul 31, 2024
touhidurabir added a commit to touhidurabir/omp that referenced this issue Jul 31, 2024
touhidurabir added a commit to touhidurabir/ops that referenced this issue Jul 31, 2024
touhidurabir added a commit that referenced this issue Jul 31, 2024
#9678 Replacing task schedular with laravel scheduler
touhidurabir added a commit to pkp/ojs that referenced this issue Jul 31, 2024
pkp/pkp-lib#9678 Replacing task schedular with laravel scheduler
touhidurabir added a commit to pkp/omp that referenced this issue Jul 31, 2024
pkp/pkp-lib#9678 Replacing task schedular with laravel scheduler
touhidurabir added a commit to pkp/ops that referenced this issue Jul 31, 2024
pkp/pkp-lib#9678 Replacing task schedular with laravel scheduler
@touhidurabir
Copy link
Member

@jonasraoni I have merged the OJS, OMP and OPS PRs but do not have the write access to plugins , details at #9678 (comment) . Can you please merge the plugins PRs ?

jonasraoni added a commit to pkp/pln that referenced this issue Jul 31, 2024
@jonasraoni
Copy link
Contributor Author

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.

asmecher added a commit to pkp/acron that referenced this issue Jul 31, 2024
asmecher added a commit to pkp/crossrefReferenceLinking that referenced this issue Jul 31, 2024
asmecher added a commit to pkp/medra that referenced this issue Jul 31, 2024
@asmecher
Copy link
Member

I've merged the 3 remaining PRs, thanks! Is this ready to be closed?

@touhidurabir
Copy link
Member

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 pkp/acron#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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Enhancement:3:Major A new feature or improvement that will take a month or more to complete. Housekeeping:1:Todo Any dependency management or refactor that would be nice to have some day.
Projects
None yet
Development

No branches or pull requests

3 participants