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

[5.7] When scheduling a job, respect the already set queue #25216

Merged

Conversation

X-Coder264
Copy link
Contributor

Resubmit of #25159 with a proper test.

@@ -97,6 +97,7 @@ public function job($job, $queue = null, $connection = null)
$job = is_string($job) ? resolve($job) : $job;

if ($job instanceof ShouldQueue) {
$queue = $queue ?? $job->queue;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any reason why you don't also consider the job connection?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just resubmitted #25159 so that it gets merged ASAP before 5.7 gets released which is basically any day now, so I didn't even consider the connection. But you are right, the connection should also be respected so I updated the PR accordingly.

@X-Coder264 X-Coder264 force-pushed the respect-set-queue-when-scheduling-a-job branch from b532761 to 5841a92 Compare August 16, 2018 13:44
@X-Coder264 X-Coder264 force-pushed the respect-set-queue-when-scheduling-a-job branch from 5841a92 to a3749b1 Compare August 16, 2018 13:48
@taylorotwell taylorotwell merged commit a3749b1 into laravel:5.7 Aug 16, 2018
@X-Coder264 X-Coder264 deleted the respect-set-queue-when-scheduling-a-job branch August 16, 2018 21:46
@whoacowboy
Copy link
Contributor

I found I had to explicitly set the connection for my SQS queues to work.

SQS would receive the message but it would continuously retry to process until it went into the dead letter queue.

This failed to process even though my default connection was 'sqs'.

            $queue_name = config('queue.connections.sqs.queue_prefix') . '_manifest';
            $job = (new LoadManifestFile($path))
            	->onQueue($queue_name);

while this worked

            $queue_name = config('queue.connections.sqs.queue_prefix') . '_manifest';
            $queue_connection = config('queue.default');
            $job = (new LoadManifestFile($path))
            	->onQueue($queue_name)
            	->onConnection($queue_connection);

Controller

    public function process(Request $request)
    {
        $path = trim($request->get('path'));
        if (isset($path)) {

            ManifestPath::updateOrCreate(['path' => $path]);
            $queue_name = config('queue.connections.sqs.queue_prefix') . '_manifest';
            $queue_connection = config('queue.default');
            $job = (new LoadManifestFile($path))
            	->onQueue($queue_name)
            	->onConnection($queue_connection);
            $this->dispatch($job);
            return ['data' => [
                'path' => trim($path)
            ]
            ];
        }
        return ['data' => [
            'success' => false
        ]
        ];

    }

Job LoadManifestFile

<?php

namespace App\Manifests\Jobs;

use Exception;
use App\Exceptions\DropboxFileDoesNotExist;
use App\Helpers\Dropbox;
use App\Manifests\Manifest;
use App\Manifests\ManifestPath;
use App\Manifests\Events\ManifestFileError;
use App\Manifests\Events\ManifestFileMoved;
use Illuminate\Bus\Queueable;
use Illuminate\Queue\SerializesModels;
use Illuminate\Queue\InteractsWithQueue;
use Illuminate\Contracts\Queue\ShouldQueue;
use Illuminate\Filesystem\FilesystemManager;
use Log;

class LoadManifestFile implements ShouldQueue
{
    use InteractsWithQueue, Queueable, SerializesModels;
    /**
     * @var
     */
    public $path;

    /**
     * Loads a manifest file into the database
     *
     * @param $path
     */
    public function __construct($path)
    {
        $this->path = $path;
    }

    /**
     * Execute the job.
     *
     * @param Dropbox $dropbox
     * @param Manifest $manifestModel
     */
    public function handle(Dropbox $dropbox)
    {
        $manifestPath = ManifestPath::where('path', $this->path)->first();
        if (isset($manifestPath)) {
            $manifestPath->delete();
        }
        try {
            $content = $this->getContent($dropbox, $this->path);
            $manifest = Manifest::firstOrCreateFromJson($content);
            $dropbox->archiveManifest($this->path,$manifest->getJson($manifest));
            broadcast(new ManifestFileMoved($this->path));
        } catch (Exception $e){
            broadcast(new ManifestFileError($this->path));
        }
    }
    private function getContent(Dropbox $dropbox, $path)
    {
        try {
            $content = $dropbox->getFileOrFail($path);
            return $content;
        } catch (DropboxFileDoesNotExist $exception){
            $this->logError($exception);
        }

    }

    /**
     * @param Exception $error
     */
    public function logError(Exception $error)
    {
        Log::info('LoadManifestFile:' . $error->getMessage());
    }
}

config/queue.php

<?php

return [

    'default' => env('QUEUE_DRIVER', 'sqs'),
    'connections' => [
        'sqs' => [
            'driver' => 'sqs',
            'key' =>  env('SQS_KEY'),
            'secret' =>  env('SQS_SECRET'),
            'prefix' => env('SQS_URL'),
            'queue_prefix' => env('SQS_QUEUE_PREFIX', 'api'),
            'queue' => env('SQS_QUEUE_PREFIX', 'api') . '_general',
            'region' => 'us-east-1',
        ],
    ],
    'failed' => [
        'database' => env('DB_CONNECTION', 'mysql'),
        'table' => 'failed_jobs',
    ],
];

@awebartisan
Copy link
Contributor

I am having the same issue with redis, it only dispatches to redis if I specify ->onConnection('redis').

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants