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

[Feature Request] Allow registering subclass of Minion::Job as task #93

Closed
reinierk opened this issue Feb 24, 2020 · 13 comments
Closed

Comments

@reinierk
Copy link

It would be nice if it is possible to register a (homebrew) subclass of Minion::Job with add_task. This would allow to factor common code into the subclass for several similar tasks.

@kraih
Copy link
Member

kraih commented Mar 23, 2020

I've been thinking about doing something like this many times. But not sure yet what the best way to implement it would be.

@kraih
Copy link
Member

kraih commented Jun 1, 2020

Whatever we end up with, i do want using job classes to be as easy as $minion->add_task(foo => 'Some::FooJobClass').

@kraih
Copy link
Member

kraih commented Jun 1, 2020

I imagine a simple job class would look like this (translated from our basic example):

package MyApp::Task::PokeMojo;
use Mojo::Base 'Minion::Job', -signatures;

sub run ($self, @args) {
  $self->app->ua->get('mojolicious.org');
  $self->app->log->debug('We have poked mojolicious.org for a visitor');
});

1;

And then you'd register it in your app with:

$minion->add_task(poke_mojo => 'MyApp::Task::PokeMojo');

And enqueue it like any other job:

$minion->enqueue('poke_mojo');

Keeping the task a Minion::Job subclass will ensure that event based extensions (using the dequeue event from Minion::Worker) keep working. And give us access to all the lower level APIs in Minion::Job for new extensions on the task level.

@rabbiveesh
Copy link

rabbiveesh commented Jun 1, 2020 via email

@kraih
Copy link
Member

kraih commented Jun 1, 2020

@rabbiveesh There will definitely not be any routing shenanigans with name translation. Multiple methods are worth considering though.

$minion->add_task(poke_mojo => 'MyApp::Task::RandomStuff#poke_mojo');

@rabbiveesh
Copy link

rabbiveesh commented Jun 1, 2020 via email

@reinierk
Copy link
Author

reinierk commented Jun 1, 2020

If you want to factor out "base" code just stick your own base class between Minion::Job and PokeMojo

@kraih
Copy link
Member

kraih commented Jun 1, 2020

One problem i do see with multiple job methods per class is that extending Minion::Job would become rather tricky. So the feature will probably not be in the first proposal.

@kraih
Copy link
Member

kraih commented Jun 2, 2020

We'll start experimenting with the feature in the next Minion release. 3fd1014...8ac422a

@shadowcat-mst
Copy link

Also, Role::Tiny exists

@reinierk
Copy link
Author

reinierk commented Jun 8, 2020

Minion::Job determines that "Run" is the run method. Fiddle with that if you want in your sub class. So I do not see that as any problem

@simonecesano
Copy link

simonecesano commented Jan 9, 2021

I've tried using the subclass feature, and found it too rigid - one subclass per task doesn't seem to offer (to me at least) great advantages compared to registering tasks in a module.

I was looking for a way to add behaviours to tasks as in a question I'd asked on stackoverflow (https://stackoverflow.com/questions/65626783/setting-the-finish-event-on-a-mojolicious-minionjob).

It turned out that adding roles to tasks looks IMHO more fluent.

The way I did it was

  • a task builder function that takes a subref as argument, plus options
  • the task builder function wraps the subref in a job, adds roles to it
  • and returns another subref that runs the job

It works ok with two roles I built - a timeout that kills the job if it runs longer than a given time, and one that posts to a URL when the job is finished or failed

app->minion->add_task(some_task => task(sub { my $job = shift; sleep 5; $job->app->log->info('I am a task'); return 'Done!'; }, { roles => { '+Alerter' => { 'alert_on' => [qw/finished failed/ ] } } }));

I am an amateur - not a pro - and new to this so bear with me pls. My code kind of sucks right now, but I could share after a little cleanup it if it helps. I found this approach more inline with the general Mojolicious/MInion way of doing things, so I thought I'd share it, I hope it's ok.

@kraih
Copy link
Member

kraih commented Feb 10, 2021

The feature is done. More additions can be discussed in new issues.

@kraih kraih closed this as completed Feb 10, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants