mojolicious / minion

:octopus: Perl high performance job queue
https://metacpan.org/release/Minion
Artistic License 2.0
224 stars 56 forks source link

[ Feature request ] An event for progress? #99

Open davehayes opened 4 years ago

davehayes commented 4 years ago

So you provide this nice thing called 'progress' in notes. That's great and it works. However, on my first (always naive) attempt to use Minion I find myself polling notes periodically because there's no event to subscribe to.

Is it possible to fire an event when a job updates notes (or notes->progress)? Failing that, is it possible to document how to use progress to actually deliver progress data in an event-driven way?

Apologies if I missed anything in the documentation that covers this. Thanks in advance.

kraih commented 4 years ago

Theoretically this would be possible to implement with PostgreSQL listen/notify, but the SQLite and MySQL backends could not provide the feature. So we are not going to do it. And it's too complex to include as a documentation example.

davehayes commented 4 years ago

Ok, so could you at least mention in the documentation that using progress requires polling the database?

kraih commented 4 years ago

How about you point out specifically the sections of the documentation that you think are too ambiguously worded? And then we'll see about rephrasing.

davehayes commented 4 years ago

Well I'm not sure it's an issue of ambiguity as much as omission. You mention progress at first in the Minion docs here:

 Minion is a high performance job queue for the Perl programming
 language, with support for multiple named queues, priorities, delayed
 jobs, job dependencies, job progress,...

Then you hint at the idea here further down in the same page:

  job
         my $job = $minion->job($id);

       Get Minion::Job object without making any changes to the actual job or
       return "undef" if job does not exist.

         # Check job state
         my $state = $minion->job($id)->info->{state};

         # Get job metadata
         my $progress = $minion->$job($id)->info->{notes}{progress};

I guess that can be generalized as metadata but as far as I can tell there's really two metadatum ... notes and results. Progress is really a subset of notes.

The word progress is not mentioned further. Digging into the documentation, there appears to be confusion in Minion::Job's docs because in the same page you first say this:

 info
         my $info = $job->info;

       Get job information.

         # Check job state
         my $state = $job->info->{state};

         # Get job metadata
         my $progress = $job->info->{notes}{progress};

but further down you say this:

note
         my $bool = $job->note(mojo => 'rocks', minion => 'too');

       Change one or more metadata fields for this job.  ...

         # Share progress information
         $job->note(progress => 95);

Perhaps I'm mistaken, but I really see 'note' as the metadata since it's just an arbitrary hash reference conceptually.

Nowhere else that I have found do I see information or examples as to how to retrieve this progress information when it changes. Maybe you could add some blurb that says "getting the notes requires polling the database for them so you can detect when they change"? I just threw that out there. ;)

rabbiveesh commented 4 years ago

I think you read into the example too much. Progress is an arbitrary meta-data field, not an inherent part of Minion.

On Wed, Aug 26, 2020 at 11:06 PM Dave Hayes notifications@github.com wrote:

Well I'm not sure it's an issue of ambiguity as much as omission. You mention progress at first in the Minion docs here:

Minion is a high performance job queue for the Perl programming language, with support for multiple named queues, priorities, delayed jobs, job dependencies, job progress,...

Then you hint at the idea here further down in the same page:

job my $job = $minion->job($id);

   Get Minion::Job object without making any changes to the actual job or
   return "undef" if job does not exist.

     # Check job state
     my $state = $minion->job($id)->info->{state};

     # Get job metadata
     my $progress = $minion->$job($id)->info->{notes}{progress};

I guess that can be generalized as metadata but as far as I can tell there's really two metadatum ... notes and results. Progress is really a subset of notes.

The word progress is not mentioned further. Digging into the documentation, there appears to be confusion in Minion::Job's docs because in the same page you first say this:

info my $info = $job->info;

   Get job information.

     # Check job state
     my $state = $job->info->{state};

     # Get job metadata
     my $progress = $job->info->{notes}{progress};

but further down you say this:

note my $bool = $job->note(mojo => 'rocks', minion => 'too');

   Change one or more metadata fields for this job.  ...

     # Share progress information
     $job->note(progress => 95);

Perhaps I'm mistaken, but I really see 'note' as the metadata since it's just an arbitrary hash reference conceptually.

Nowhere else that I have found do I see information or examples as to how to retrieve this progress information when it changes. Maybe you could add some blurb that says "getting the notes requires polling the database for them so you can detect when they change"? I just threw that out there. ;)

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/mojolicious/minion/issues/99#issuecomment-681096081, or unsubscribe https://github.com/notifications/unsubscribe-auth/AFURPKR7ZCPOTYFCR27P2W3SCVTMNANCNFSM4QMGAHLQ .

davehayes commented 4 years ago

Minion explicitly states that it supports job progress, so it's natural to me to go looking for where that support is.

kraih commented 4 years ago

Yes, you're reading too much into the example. The notes feature is job progress information, there is no subset.

kraih commented 4 years ago

I think what it all comes down to is that the Minion guide explaining all the features with examples is still missing.

kraih commented 4 years ago

The guide is now a planned feature, but i have no idea when i'll get around to actually writing it. https://github.com/mojolicious/minion/issues/100

davehayes commented 4 years ago

A guide which explains all the features with examples would address my concerns. IMHO if I have to look at your code to understand what something actually does, the documentation is by definition lacking in that area.

Not that I don't like looking at your code either, you have a uniquely terse style that can be fun to figure out...and not that people who are using your code shouldn't look at it anyway...but this is strictly a measure of documentation quality.

davehayes commented 4 years ago

I've addressed the event issue myself because I use the Pg backend. What would it take to get support for listen/notify in SQLite or MySQL? Can you do listen/notify without Pg? I'm just curious here.

Grinnz commented 4 years ago

It's not possible with SQLite because there is no server.

rabbiveesh commented 4 years ago

You would need to use something which has a pub/sub interface. Options include a websocket hub (like Mercury) or Redis (if you want to stay strictly in the Mojo camp).

On Thu, Aug 27, 2020 at 12:14 AM Dave Hayes notifications@github.com wrote:

I've addressed the event issue myself because I use the Pg backend. What would it take to get support for listen/notify in SQLite or MySQL? Can you do listen/notify without Pg? I'm just curious here.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/mojolicious/minion/issues/99#issuecomment-681127378, or unsubscribe https://github.com/notifications/unsubscribe-auth/AFURPKTQOZWQE4CAZ4JUIWTSCV3KNANCNFSM4QMGAHLQ .

davehayes commented 4 years ago

It's not possible with SQLite because there is no server.

I'm just covering all the bases here. I recognize this, but maybe someone has written something that does pubsub for SQLite or even for nothing at all (rabbiveesh's comment ... redis for example). I'm trying to work within the bounds of what has been stated to get the feature I feel should likely be a part of Minion.

So to my original request ... not a wanted feature then? I can live with that as long as every other option has been exhausted. Maybe you could make pubsub event an optional feature of Minion::Backend::Pg?

rabbiveesh commented 4 years ago

You're working with too blunt of a hammer. You're assuming that EVERY item put in notes is related to progress, which isn't necessarily true. For example, I personally use notes to cache progress in a potentially flaky job, for example. No need for any pubsub there.

So to my original request ... not a wanted feature then? I can live with

that as long as every other option has been exhausted. Maybe you could make pubsub event an optional feature of Minion::Backend::Pg?

No, kraih said that he won't add features unique to any specific backend. You can write a role which wraps note and does the pubsub there. That should be fairly trivial. It just can't be a builtin feature.

On Thu, Aug 27, 2020 at 12:24 AM Dave Hayes notifications@github.com wrote:

It's not possible with SQLite because there is no server.

I'm just covering all the bases here. I recognize this, but maybe someone has written something that does pubsub for SQLite or even for nothing at all (rabbiveesh's comment ... redis for example). I'm trying to work within the bounds of what has been stated to get the feature I feel should likely be a part of Minion.

So to my original request ... not a wanted feature then? I can live with that as long as every other option has been exhausted. Maybe you could make pubsub event an optional feature of Minion::Backend::Pg?

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/mojolicious/minion/issues/99#issuecomment-681131780, or unsubscribe https://github.com/notifications/unsubscribe-auth/AFURPKXN62GJAN5XNZLXY2LSCV4PNANCNFSM4QMGAHLQ .

davehayes commented 4 years ago

You're working with too blunt of a hammer. You're assuming that EVERY item put in notes is related to progress, which isn't necessarily true.

You might be misunderstanding me. I see this:

         # Get job metadata
         my $progress = $job->info->{notes}{progress};

and I assume progress is a subset of notes. I even said that:

I guess that can be generalized as metadata but as far as I can tell there's really two metadatum ... notes and results. Progress is really a subset of notes

Not sure what you are perceiving here. :)

rabbiveesh commented 4 years ago

Notes is a hashref. I don't know what you mean by subset.

On Thu, Aug 27, 2020 at 12:34 AM Dave Hayes notifications@github.com wrote:

You're working with too blunt of a hammer. You're assuming that EVERY item put in notes is related to progress, which isn't necessarily true.

You might be misunderstanding me. I see this:

     # Get job metadata
     my $progress = $job->info->{notes}{progress};

and I assume progress is a subset of notes. I even said that:

I guess that can be generalized as metadata but as far as I can tell there's really two metadatum ... notes and results. Progress is really a subset of notes

Not sure what you are perceiving here. :)

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/mojolicious/minion/issues/99#issuecomment-681136035, or unsubscribe https://github.com/notifications/unsubscribe-auth/AFURPKTEVIHSJWG3NI5S3XTSCV5XDANCNFSM4QMGAHLQ .

davehayes commented 4 years ago

So if notes is a hashref and progress can be a hashref (it always is in my case) and progress/hashref is a key/value pair in notes... in that case is it not valid to say that progress is a subset of notes?

kraih commented 4 years ago

You might want to continue that discussion in the Google Group or on IRC.