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

Update LumenServiceProvider.php #9

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

JordanDalton
Copy link

Let the developer determine when the job gets deleted. If $event->job->delete() was kept jobs would be deleted regardless if handle() was successful.

Let the developer determine when the job gets deleted. If $event->job->delete() was kept jobs would be deleted regardless if handle() was successful.
@robertwe
Copy link

+1 Can we merge it ?

@cristiangrama
Copy link

@JordanDalton Not sure this will work. If you remove that part your job will no longer be deleted. In AWS world, the job will stay in the queue and when the visibility timeout expires you're gonna process the job again. Then in all your Jobs that are plain you'll need to manually delete the job.

The standard queue driver uses the CallQueueHandler which will delete the job once it's done (https://github.com/illuminate/queue/blob/master/CallQueuedHandler.php#L57). I guess that the issue here is that the sqs-plain driver does not use a separate handler that would delete the job once done and that's why you need that after event to ensure the deletion. Not the greatest.

Here's a different solution that I have implemented in my own Lumen service provider:

        /** @var QueueManager $queueManager */
        $queueManager = $this->app->make('queue');

        $queueManager->after(function (JobProcessed $event) {
            if ($event->job->getConnectionName() === 'sqs-plain') {
                $event->job->delete();
            }
        });

The above ensures that if you use both the standard and plain queue you'll not get your standard queue job deleted if you instead wanted to release it back to the queue.

To fix it fully I suppose that I could add another check in there to match the queued handler check where I only delete it if it's not already deleted or released back to the queue.

        /** @var QueueManager $queueManager */
        $queueManager = $this->app->make('queue');

        $queueManager->after(function (JobProcessed $event) {
            if ($event->job->getConnectionName() === 'sqs-plain' &&
                ! $event->job->isDeletedOrReleased()
            ) {
                $event->job->delete();
            }
        });

I have not tried yet the second option but that should work I guess.

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.

3 participants