mvniekerk / tokio-cron-scheduler

Schedule tasks on Tokio using cron-like annotation
Apache License 2.0
529 stars 59 forks source link

Integrate english-to-cron crate #79

Closed kaplanelad closed 2 months ago

kaplanelad commented 2 months ago

It would be beneficial to integrate the english-to-cron crate into your library to allow users to generate cron expressions using plain English text. This would enhance the usability of your library by making it easier for developers who are less familiar with cron syntax to define scheduling rules.

if this is something that you want, I'd love to open a PR for that

mvniekerk commented 2 months ago

Hi @kaplanelad I've tried to do it in the english-to-cron-integration branch in a non-destructive way. It seems like the existing croner parsing is failing after using this.

kaplanelad commented 2 months ago

Hey @mvniekerk, Do you have a text example that converted wrong?

mvniekerk commented 2 months ago

If you run the simple example with the English feature enabled

Groete / Best regards Michael van Niekerk


Van: Elad Kaplan @.> Gestuur: Saturday, September 21, 2024 6:29:45 PM aan: mvniekerk/tokio-cron-scheduler @.> Aa: Michael van Niekerk @.>; Mention @.> onderwerp: Re: [mvniekerk/tokio-cron-scheduler] Integrate english-to-cron crate (Issue #79)

Hey @mvniekerkhttps://github.com/mvniekerk, Do you have a text example that converted wrong?

— Reply to this email directly, view it on GitHubhttps://github.com/mvniekerk/tokio-cron-scheduler/issues/79#issuecomment-2365244919, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AAL7O742KYMAJTFAXSY6XJLZXWNHTAVCNFSM6AAAAABOH4PGKCVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDGNRVGI2DIOJRHE. You are receiving this because you were mentioned.Message ID: @.***>

kaplanelad commented 2 months ago

I noticed that you added the text every 10 seconds, which the library correctly converts into the cron syntax 0/10 * * * * ? *.

I have also tested the cron expression 0/10 * * * * ? * directly with the library, and it executes as expected.

kaplanelad commented 2 months ago

It seems the syntax generator is functioning properly. However, when I debug it on your branch, I encounter the following error:

Pattern must consist of five fields, seconds are not allowed by configuration.

the english-to-cron syntax return pattern with 7, including the seconds. This is also documented in the readme that the patterns should include the following format:

sec   min   hour   day of month   month   day of week   year
*     *     *      *              *       *             *
mvniekerk commented 2 months ago

Ok, so it seems like croner-rust doesn't like the year field. I've noticed you're the author of english-to-cron - thank you for this! v0.13.0 will have the crate included

kaplanelad commented 2 months ago

@mvniekerk, why did you add it as a feature flag and as part of the base features? this crate is very small

mvniekerk commented 2 months ago

Hi Elad I had previous bug reports of minimizing crate imports and feature gating, so this goes with that spirit. Since this pulls in extra crates and adds parsing complexity, I’d rather give people what they had and not something they didn’t asked for. I do think that the existence of the feature is quite obvious in the README, which will make it more discoverable.

From: Elad Kaplan @.> Date: Sunday, 22 September 2024 at 14:22 To: mvniekerk/tokio-cron-scheduler @.> Cc: Michael van Niekerk @.>, Mention @.> Subject: Re: [mvniekerk/tokio-cron-scheduler] Integrate english-to-cron crate (Issue #79)

@mvniekerkhttps://github.com/mvniekerk, why did you add it as a feature flag and as part of the base features? this crate is very small

— Reply to this email directly, view it on GitHubhttps://github.com/mvniekerk/tokio-cron-scheduler/issues/79#issuecomment-2366762352, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AAL7O7YYJNQZEXM4XEYRTL3ZX2ZBZAVCNFSM6AAAAABOH4PGKCVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDGNRWG43DEMZVGI. You are receiving this because you were mentioned.Message ID: @.***>

kaplanelad commented 2 months ago

I got it, and it sounds reasonable. Thanks!