dmanto / Mojolicious-Plugin-Cron

a Cron-like helper for Mojolicious full and lite apps
Other
3 stars 7 forks source link

[Feature request] Make the subs allowed to be async #11

Closed akarelas closed 3 years ago

akarelas commented 3 years ago

Could you make the sub callbacks allowed to be async functions? (see Future::AsyncAwait)

This would allow subs that have a long duration (such as massive cleanup subs) to be called by cron, without interrupting the real-time nature of the Mojolicious framework.

I could write a PR for you if you want. It's only a few lines of code, I believe, it's not hard to do.

dmanto commented 3 years ago

Ok, please prepare a PR and I'll take a look at it.

Also note that the current implementation does allow non-blocking operations (through callbacks)

akarelas commented 3 years ago

You're right. My idea is not really needed, is it? You already can use async subs.

dmanto commented 3 years ago

Probably not, but I just realized after your last answer :)

Mojo framework evolved from callbacks only to recommend promises and finally the use of async / await.

As a first step we could upgrade the plugin to have a "promises like" solution, converting it to a "thennable" plugin, like

plugin Chron_p ('* * ...')->then(sub {})...

But I don't see the real benefit here, as probably many of the advantages promises allow, like ->race, or ->all, doesn't really make sense to me for crons.

For async / await is probably worst. I imagine async / await naturally fitting on subs that only run one time after called. Also note that you normally establish all the crons that you may need at the begining of the app, before calling the start method. So if you 'await' there for some cron in the future, the hole program will wait, despite you are not blocking the ioloop, but still, nothing will happen until the cron fires the first time.

But as you said, inside the subs called by Cron you can use wathever you need (as long you don't block the ioloop).

For long processes like the ones you mentioned in the issue, what I do is trigger a Minion job. That will not block your ioloop.

akarelas commented 3 years ago

I should close this issue, right?