enupal / backup

Fully integrated Backup solution for Craft CMS
https://enupal.com/craft-plugins/enupal-backup
Other
16 stars 3 forks source link

Manually calling getQueue()->run() has problems with different queue drivers #29

Closed angrybrad closed 4 years ago

angrybrad commented 4 years ago

Description

The Yii 2 queue package has drivers for lots of different queue services.

https://github.com/yiisoft/yii2-queue

If you've configured Craft to use one of them (SQS for example), you'll get the following error:

ArgumentCountError
Too few arguments to function yii\queue\sqs\Queue::run(), 0 passed in /var/app/current/vendor/enupal/backup/src/services/Backups.php on line 98 and at least 1 expected
1. in /var/app/current/vendor/yiisoft/yii2-queue/src/drivers/sqs/Queue.phpat line 88
79808182838485868788899091929394959697 
    /**
     * Listens queue and runs each job.
     *
     * @param bool $repeat whether to continue listening when queue is empty.
     * @param int $timeout number of seconds to sleep before next iteration.
     * @return null|int exit code.
     * @internal for worker command only
     */
    public function run($repeat, $timeout = 0)
    {
        return $this->runWorker(function (callable $canContinue) use ($repeat, $timeout) {
            while ($canContinue()) {
                if (($payload = $this->reserve($timeout)) !== null) {
                    $id = $payload['MessageId'];
                    $message = $payload['Body'];
                    $ttr = (int) $payload['MessageAttributes']['TTR']['StringValue'];
                    $attempt = (int) $payload['Attributes']['ApproximateReceiveCount'];
                    if ($this->handleMessage($id, $message, $ttr, $attempt)) {
2. in /var/app/current/vendor/enupal/backup/src/services/Backups.php at line 98– yii\queue\sqs\Queue::run()
9293949596979899100101102103104            }
        } else {
            // if is Linux try to call queue/run
            if ($settings->runJobInBackground){
                $this->runQueueInBackground();
            }else{
                Craft::$app->getQueue()->run();
            }
            $response['message'] = 'running';
        }

        return $response;
    }
3. in /var/app/current/vendor/enupal/backup/src/controllers/BackupsController.php at line 182– enupal\backup\services\Backups::executeEnupalBackup()

This comes from https://github.com/enupal/backup/blob/master/src/services/Backups.php#L98

The QueueInterface class that specifies the run method is Craft's class that the Yii2 queue libraries have no knowledge of.

I guess I'm not seeing why you wouldn't let the user/Craft decide when to process the queue.

Generally speaking, you don't want to manually trigger Craft's queue to run. i.e. on most installs, if someone triggers a manually backup from Craft's control panel, it'll start getting processed via the web-based queue runner when the page refreshes.

And if someone has set up a daemon or cron job to process Craft's queue, it'll get triggered immediately or whenever the next cron job is scheduled to run.

Seems like this plugin should just be pushing the job to the queue and calling it a day?

Additional info

brandonkelly commented 4 years ago

Your UI can also manually trigger Craft’s Ajax-based queue runner by calling Craft.cp.runQueue(); from your JavaScript. (You don’t need to factor in how the queue worker is set up when calling that.)

andrelopez commented 4 years ago

Hi @brandonkelly @angrybrad Thanks for the tips. Enupal Backup v1.5.0 is out and fixed manually calls to the queue->run and also recommends to users to add a cron job to call craft queue/run whenever they need scheduled backups.

brandonkelly commented 4 years ago

Thanks @andrelopez !