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.4] allow scheduling of queued jobs #18235

Merged
merged 3 commits into from Mar 7, 2017
Merged

Conversation

tomschlick
Copy link
Contributor

As proposed in laravel/ideas#457, this simple addition allows us to schedule a job to be queued at a specific interval.

This enables you do have something like this

# /app/Console/Kernel.php

$schedule->job(new Heartbeat())->everyFiveMinutes();

In this example, you might be scheduling a heartbeat to be run from the queue workers themselves. This is something I currently do in production which ping's Envoyer's heartbeat endpoints to tell me if a queue is backed up for too long.

Right now, if you want to do this you have to either create a dedicated command per job you want to schedule, or use the $schedule->call(); method passing in a closure which can be overly verbose for the Kernel file. This provides a clean abstraction to that.

Copy link
Contributor

@leo108 leo108 left a comment

Choose a reason for hiding this comment

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

Since this is a new feature, I think it's better to do the changes base on master branch

* @param \Illuminate\Contracts\Queue\ShouldQueue $job
* @return \Illuminate\Console\Scheduling\Event
*/
public function job(ShouldQueue $job)
Copy link
Contributor

Choose a reason for hiding this comment

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

Why not add a $name = null as the second parameter?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Name is used solely for the cache mutex name. I don't see a case where you'd want to set that as anything other than the full class name of the Job.

Copy link
Contributor

Choose a reason for hiding this comment

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

@tomschlick my bad, I mixed it up with the scheduled command name

@tomschlick
Copy link
Contributor Author

Features that don't have compatibility usually go on the current branch in this repo. Master is for 5.5.

@taylorotwell taylorotwell merged commit 0a9d6d0 into laravel:5.4 Mar 7, 2017
@taylorotwell
Copy link
Member

Merged with a few tweaks... I removed the ShouldQueue type-hint in case people are using jobs in a synchronous "command-bus" fashion, also allowed passing a string class name which will resolve the job from the container. Thanks!

@tomschlick
Copy link
Contributor Author

sounds good, thanks!

@tomschlick tomschlick deleted the patch-3 branch March 7, 2017 14:48
@LasseRafn
Copy link
Contributor

Wow... no more commands that queues jobs 🎉👍🏻

@dmason30
Copy link
Contributor

dmason30 commented Sep 7, 2017

@taylorotwell This should be added to the docs. Had to find this PR to find out if it this functionality existed. Doesn't appear to be in 5.4/5.5 docs.

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.

None yet

5 participants