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

Widen cron syntax support, update docs #32414

Open
rarkins opened this issue Nov 8, 2024 · 6 comments · May be fixed by #32718
Open

Widen cron syntax support, update docs #32414

rarkins opened this issue Nov 8, 2024 · 6 comments · May be fixed by #32718
Assignees
Labels
core:schedule Relating to the "schedule" field and behavior priority-2-high Bugs impacting wide number of users or very important features

Comments

@rarkins
Copy link
Collaborator

rarkins commented Nov 8, 2024

Describe the proposed change(s).

Discussion: #32369

We should support as much of the cron spec as our parser allows us. I'm not sure why we needed to write our own logic actually, isn't there the ability for us to use a library to tell us "is this now?"

Additionally, our docs on scheduling should be changed to be cron-first and not using later syntax by default.

@rarkins rarkins added priority-2-high Bugs impacting wide number of users or very important features core:schedule Relating to the "schedule" field and behavior labels Nov 8, 2024
@RahulGautamSingh
Copy link
Collaborator

RahulGautamSingh commented Nov 17, 2024

We could use the cron-parser library's next() method as implemented in this PR. But, this only covers use of L in the day of month and day of week field. And some other keywords like #, W are not supported yet by this library.

Some other libraries which also have the equivalent of above mentioned next() method are:

  1. cronosjs (supports every cron special char)
  2. croner ( doesn't support W)
  3. node-schedule (does not support L, W and #)
  4. cron-schedule (even has a matchDate fn, like we want ... but doesn't support L and #)

@rarkins
Copy link
Collaborator Author

rarkins commented Nov 17, 2024

Take the one which supports all?

@RahulGautamSingh
Copy link
Collaborator

Take the one which supports all?

Cronosjs does but has 60k downloads...whereas Croner only falls short on W char but has 2mil downloads .

I am confused between these two.

@rarkins
Copy link
Collaborator Author

rarkins commented Nov 17, 2024

Cronojs seems "unmaintained", although with no bugs open either. I think it's worth going with a maintained dependency, even if we have to document 1-2 limitations

@RahulGautamSingh
Copy link
Collaborator

RahulGautamSingh commented Nov 24, 2024

Additionally, our docs on scheduling should be changed to be cron-first and not using later syntax by default.

Should all the schedule examples in later syntax be converted?

@rarkins
Copy link
Collaborator Author

rarkins commented Nov 24, 2024

Yes good idea

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
core:schedule Relating to the "schedule" field and behavior priority-2-high Bugs impacting wide number of users or very important features
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants