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

Add Cron module and Schedule.cron constructor #1855

Merged
merged 27 commits into from
Jan 9, 2024
Merged

Conversation

fubhy
Copy link
Member

@fubhy fubhy commented Jan 3, 2024

  • Add cron expression parser
  • Add cron match calculator
  • Add cron next calculator
  • Add Schedule.cron constructor
  • Write tests for Schedule.cron

Fixes #1524

Copy link

changeset-bot bot commented Jan 3, 2024

🦋 Changeset detected

Latest commit: 1c8ebbf

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 16 packages
Name Type
effect Minor
@effect/cli Major
@effect/opentelemetry Major
@effect/platform-browser Major
@effect/platform-bun Major
@effect/platform-node Major
@effect/platform Major
@effect/printer-ansi Major
@effect/printer Major
@effect/rpc-http-node Major
@effect/rpc-http Major
@effect/rpc-nextjs Major
@effect/rpc-workers Major
@effect/rpc Major
@effect/schema Major
@effect/typeclass Major

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@fubhy fubhy marked this pull request as draft January 3, 2024 13:09
@github-actions github-actions bot changed the base branch from main to next-minor January 3, 2024 13:09
@fubhy fubhy marked this pull request as ready for review January 4, 2024 20:21
Comment on lines +429 to +435
if (now < previous[0]) {
return core.succeed([
[false, previous],
[previous[1], previous[2]],
ScheduleDecision.continueWith(Interval.make(previous[1], previous[2]))
])
}
Copy link
Member Author

@fubhy fubhy Jan 5, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is an optimization that makes this work with composed schedules without the overhead of re-calculating the next Cron interval repeatedly (e.g. when composing this with sth. like Schedule.fixed at second intervals "run every second on the first minute of the second hour of each monday between january and march" :-P)

@mikearnaldi
Copy link
Member

Is there any usage expectation of Cron apart from Schedule? if not it could be an internal module

@fubhy
Copy link
Member Author

fubhy commented Jan 8, 2024

Is there any usage expectation of Cron apart from Schedule? if not it could be an internal module

Would that be your preference? I don't have a strong preference either way. I can /see/ this being useful outside of Schedule in user land but I don't have an immediate use-case myself so would be happy either way.

There are several cron npm packages with a few million weekly downloads:

Hard to say for what these are mostly used.

I don't see much harm in maintaining this as a public module other than the maintenance burden. Your call ;-)

@mikearnaldi
Copy link
Member

Is there any usage expectation of Cron apart from Schedule? if not it could be an internal module

Would that be your preference? I don't have a strong preference either way. I can /see/ this being useful outside of Schedule in user land but I don't have an immediate use-case myself so would be happy either way.

There are several cron npm packages with a few million weekly downloads:

Hard to say for what these are mostly used.

I don't see much harm in maintaining this as a public module other than the maintenance burden. Your call ;-)

My question was more out of curiosity, I agree regarding the maintenance burden, if this is stable it can be publicly exposed, only downside is if this is not stable and leads to breaking changes. I don't have a strong opinion, we can start having it public for now and reserve the right to decide otherwise if we feel like.

@mikearnaldi mikearnaldi merged commit 3abb2f6 into next-minor Jan 9, 2024
9 checks passed
@mikearnaldi mikearnaldi deleted the schedule-cron branch January 9, 2024 12:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Consider adding support for cron expressions for Schedule
2 participants