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

feat(every): adding every N minutes or hours #31

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

thoven87
Copy link
Contributor

This PR adds the followings:

Run a job every 5 minutes or 15, 30, 45 and so forth. Please note that all minutes or hours always start at 0

var jobScheduleService = JobSchedule()
jobScheduleService.addJob(
        ScheduledJobController.StatsParameters(count: 20),
        schedule: .every(minute: 5)
)

The same is true for every N hour

var jobScheduleService = JobSchedule()
jobScheduleService.addJob(
        ScheduledJobController.StatsParameters(count: 20),
        schedule: .every(hour: 3)
)

Copy link
Member

@adam-fowler adam-fowler left a comment

Choose a reason for hiding this comment

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

Two things

  • This should be two functions, one for minutes and one for hours
  • The function promises something it can't deliver ie generate an event every so many minutes. This only works for factors of 60 or 24 depending on value.

Perhaps you should have an enum for the factors eg

enum MinuteFactor: Int {
  case two = 2
  case three = 3
  case four = 4
  case five = 5
  case six = 6
  case ten = 10
  case twelve = 12
  case quarterHour = 15
  case twenty = 20
  case halfHour = 30
}
func everyMinute(_ minute: MinuteFactor)

And something similar for hours. For the hours you could also add a start time eg

func everyHour(_ hour: HourFactor, startingHour: Int, minute: Int)

@thoven87
Copy link
Contributor Author

Two things

  • This should be two functions, one for minutes and one for hours
  • The function promises something it can't deliver ie generate an event every so many minutes. This only works for factors of 60 or 24 depending on value.

Perhaps you should have an enum for the factors eg

enum MinuteFactor: Int {
  case two = 2
  case three = 3
  case four = 4
  case five = 5
  case six = 6
  case ten = 10
  case twelve = 12
  case quarterHour = 15
  case twenty = 20
  case halfHour = 30
}
func everyMinute(_ minute: MinuteFactor)

And something similar for hours. For the hours you could also add a start time eg

func everyHour(_ hour: HourFactor, startingHour: Int, minute: Int)

I need to re-think this PR a little bit. There's currently func called everyMinute which takes seconds, but does not work with fractions i.e everyMinute(second: 300) which would translate into every 5 minutes starting at 0-5-10-15-20...55

Would it be easier to expose a func that takes in a cron syntax and computes the schedule?

@adam-fowler
Copy link
Member

I need to re-think this PR a little bit. There's currently func called everyMinute which takes seconds, but does not work with fractions i.e everyMinute(second: 300) which would translate into every 5 minutes starting at 0-5-10-15-20...55

The seconds parameter in everyMinute is when in the minute the schedule triggers a job. It is probably unnecessary for this function, but for everyHour or everyDay it is important to be able to say when in the hour or day we want the job to trigger. everyMinute(second: 300) basically means never as the second value can only be between 0 and 59.

Would it be easier to expose a func that takes in a cron syntax and computes the schedule?

We don't support the full crontab syntax eg we don't support * 4 * * * ie every minute when the when the hour is 4

@thoven87
Copy link
Contributor Author

I need to re-think this PR a little bit. There's currently func called everyMinute which takes seconds, but does not work with fractions i.e everyMinute(second: 300) which would translate into every 5 minutes starting at 0-5-10-15-20...55

The seconds parameter in everyMinute is when in the minute the schedule triggers a job. It is probably unnecessary for this function, but for everyHour or everyDay it is important to be able to say when in the hour or day we want the job to trigger. everyMinute(second: 300) basically means never as the second value can only be between 0 and 59.

Would it be easier to expose a func that takes in a cron syntax and computes the schedule?

We don't support the full crontab syntax eg we don't support * 4 * * * ie every minute when the when the hour is 4

Understood, I thought of writing a Cron parser and expose mundane syntax such as

@daily
@weekly
@monthly
@yearly
@minute

and finally

* 4 * * *
5 * * * *
* * * * * 

What do you think?

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.

2 participants