Explicitly specify job queue #147
Conversation
| #[Override] | ||
| protected function tearDown(): void | ||
| { | ||
| parent::tearDown(); | ||
| #[Override] | ||
| protected function tearDown(): void | ||
| { | ||
| parent::tearDown(); | ||
|
|
||
| CloudTasksQueue::forgetWorkerOptionsCallback(); | ||
| } | ||
| CloudTasksQueue::forgetWorkerOptionsCallback(); | ||
| } |
There was a problem hiding this comment.
I ran composer pint, thus this change.
| @@ -82,7 +84,7 @@ protected function setConfigValue($key, $value) | |||
| $this->app['config']->set('queue.connections.my-cloudtasks-connection.'.$key, $value); | |||
| } | |||
|
|
|||
| public function dispatch($job): DispatchedJob | |||
| public function dispatch($job, ?string $onQueue = null): DispatchedJob | |||
There was a problem hiding this comment.
Added an optional parameter that allows to set queue for a closure job.
| && ($command['queue'] ?? null) === null | ||
| && $command['queue'] === 'barbequeue' |
There was a problem hiding this comment.
This failed initially after the changes, because $command['queue'] is now always set.
| @@ -146,17 +147,22 @@ public function it_posts_the_task_the_correct_queue() | |||
| // Arrange | |||
| CloudTasksApi::fake(); | |||
|
|
|||
| $closure = fn () => 'closure job'; | |||
| $closureDisplayName = CallQueuedClosure::create($closure)->displayName(); | |||
There was a problem hiding this comment.
Display name of a closure job looks like Closure (QueueTest.php:150). Added this line to keep the ability to assert displayName in CloudTasksApi::assertTaskCreated().
|
Sorry for pinging you @marickvantuil, but would you be able to look into this at your earliest convenience? |
marickvantuil
added
the
safe-to-test
|
Thanks! I'll check quickly if I can make a test for the batched job. Might have to rewrite some stuff in the TestCase. Apologies for the later reply - I was on holiday :-) |
|
Thanks for taking your time to look into this! |
Issue
Take this batch for example:
Running
php artisan queue:work --queue=batchin a regular Laravel application successfully executesTestBatchedJobjob onbatchqueue, even if the job has set a different queue, such as$this->queue = 'default'.Using the same batch with this package is currently not possible, unless you specifically set the same queue on the job too. Batch jobs do get dispatched to GCT on correct queue (
batch), but it fails the execution because it cannot pass existence check onTaskHandler. That's because it tries to resolve queue name from serialized payload, but it won't find one and default todefaultqueue set in config, unless you specified queue directly on the job.Solution
I found out that we could always set the queue on the job instance before dispatching it to GCT. Unfortunately it's not possible to to do that for batched closures, because they are transformed into
CallQueuedClosureinside\Illuminate\Queue\Queue::createPayload()and then immediately serialized.Only 1 test failed initially with this change and it was because it was expecting
SimpleJobqueue to benull, but with this change it is set to default queue. I added couple extra cases to test regular closure dispatch too. . I haven't found any batch tests in this repo, so I'm not sure how to best test batch dispatching to showcase the original issue.