Add Skip First Run option

Rykian / clockwork

A scheduler process to replace cron.

MIT License

544 stars 66 forks source link

Add Skip First Run option #8

Closed ChaoticBoredom closed 7 years ago

ChaoticBoredom commented 7 years ago

Working w/ clockwork on Heroku, we often want to delay the first run of some jobs on deploys. Usually things like metric collection, particularly those triggering alerts. If they are run at the moment of deploy, they return bad data. We'd like to delay the first run of these alerts for a single cycle. I'm putting this PR up, not sure how in demand this particular feature is, but I'm open to alternatives if this isn't the route the maintainer wants to go.

mikker commented 7 years ago

Any plans to merge this? Been using it for a while with no trouble.

MarcLapierre commented 7 years ago

👍 I would love to see this merged. It was not immediately obvious that all jobs would be run on deploy even if at is set. This is causing issues for us as we have a lot of staggered high intensity jobs that we do not want to run on each deploy as we deploy several times per day.

MarcLapierre commented 7 years ago

We've tried this out over the last few days and we have found the following issues:

This breaks clockwork-test. Calling Clockwork::Test.run(max_ticks: 1) causes this line to lockup forever.
Any scheduled jobs that use at: will never trigger.

ChaoticBoredom commented 7 years ago

Thanks Marc, will look into fixes for those issues later today :)

kevin-j-m commented 7 years ago

Maintainer of clockwork-test here. I'm on vacation, so I can't take a look now at the correlation that causes the gem to fail with this change.

However, that's on us to figure out & maintain, so don't let it hold up development here. PRs or advise on maintaining compatibility are always welcome!

Thanks for taking a look at it @MarcLapierre.

MarcLapierre commented 7 years ago

Hey guys -- I am looking for a follow-up here. Are we looking at resolving and merging this?

ChaoticBoredom commented 7 years ago

@kevin-j-m thanks, I misread that comment, thought it was breaking tests in the main clockwork gem. I'm not sure how to move this forward at this point :/

Rykian commented 7 years ago

@ChaoticBoredom Could you add some documentation to the README ? Thanks ;)

ChaoticBoredom commented 7 years ago

@Rykian sorry for the delay, updated the README, let me know if more detail is needed, or if there's a section I've missed :)

mikker commented 7 years ago

Let's go! ☺️

MarcLapierre commented 7 years ago

Hey @ChaoticBoredom -- Did we ever resolve the issue around Any scheduled jobs that use at: will never trigger? I don't see any commits regarding that issue.

ChaoticBoredom commented 7 years ago

@MarcLapierre sorry for the delay, that was not addressed. Also, until today I misunderstood how clockwork worked using :at, so I'll put something up to handle that.